Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #154 process_response return ResponseError #155

Merged
merged 1 commit into from Jan 3, 2024

Conversation

DanGould
Copy link
Contributor

This returns an option so that empty OK responses from the directory can poll again for a response.

The main v1 integration test handling these errors already seems sufficient to me. Ideally these v1/v2 code paths merge once we get achieve the Payjoin V2 Beta milestone.

Ok(Some(psbt)) => return Ok(psbt),
Ok(None) => std::thread::sleep(std::time::Duration::from_secs(5)),
Err(re) => {
println!("{}", re);
Copy link
Collaborator

Choose a reason for hiding this comment

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

extra log here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it? RE's display and debug impls are distinct

Copy link
Collaborator

Choose a reason for hiding this comment

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

why would we need both? aint the debug verbose enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

debug is verbose enough but in normal operation you the debug log is not printed to stdout. That only happens when debugging and manually setting RUST_LOG=debug

@@ -195,6 +195,7 @@ impl App {

#[cfg(feature = "v2")]
async fn long_poll_post(&self, req_ctx: &payjoin::send::RequestContext) -> Result<Psbt> {
use payjoin::send::ResponseError;
Copy link
Contributor Author

@DanGould DanGould Dec 29, 2023

Choose a reason for hiding this comment

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

unused import @jbesraa soz I overlooked this

@DanGould
Copy link
Contributor Author

DanGould commented Jan 2, 2024

@jbesraa I really want to merge this fix before we do the release. Please review, I ran cargo clippy and found that unused import

This was referenced Jan 2, 2024
@DanGould DanGould merged commit 96212cc into payjoin:master Jan 3, 2024
5 checks passed
@DanGould DanGould deleted the fix-response-err branch January 3, 2024 16:04
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.

None yet

2 participants