Skip to content

Handle fetch_ohttp_keys response status#576

Merged
DanGould merged 1 commit intopayjoin:masterfrom
DanGould:check-fetch-status
Mar 13, 2025
Merged

Handle fetch_ohttp_keys response status#576
DanGould merged 1 commit intopayjoin:masterfrom
DanGould:check-fetch-status

Conversation

@DanGould
Copy link
Contributor

Only parse success responses as OhttpKeys. Other responses may be errors to be propagated.

I caught this failure while testing ohttp-relay with an http (not https) proxy, which throws a 400 Bad Request error, but the fetch code only displayed "Invalid ohttp keys returned from payjoin directory"

@DanGould DanGould force-pushed the check-fetch-status branch from ab4f8a2 to 5d28bbc Compare March 13, 2025 17:30
@DanGould DanGould requested a review from spacebear21 March 13, 2025 17:31
@coveralls
Copy link
Collaborator

coveralls commented Mar 13, 2025

Pull Request Test Coverage Report for Build 13843423303

Details

  • 49 of 56 (87.5%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.09%) to 80.631%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin/src/io.rs 49 56 87.5%
Totals Coverage Status
Change from base Build 13839302301: 0.09%
Covered Lines: 4833
Relevant Lines: 5994

💛 - Coveralls

@spacebear21
Copy link
Collaborator

Is this failure unit-testable?

@DanGould DanGould force-pushed the check-fetch-status branch from 5d28bbc to 41865a1 Compare March 13, 2025 19:52
Only parse success responses as OhttpKeys. Other responses may be errors
to be propagated. These get a new UnexpectedStatusCode error variant.

I caught this failure while testing ohttp-relay with an http (not https)
proxy, which throws a 400 Bad Request error, but the fetch code only
displayed "Invalid ohttp keys returned from payjoin directory"
@DanGould
Copy link
Contributor Author

added unit tests

Copy link
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 41865a1

@DanGould DanGould merged commit 13e8c6b into payjoin:master Mar 13, 2025
7 checks passed
@DanGould DanGould deleted the check-fetch-status branch March 13, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants