Skip to content

Retry pj on version error #151

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.

Sign up for GitHub

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

Closed
jbesraa wants to merge 2 commits into from
Closed

Retry pj on version error #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7af9c6c
Impl `send::ResponseError::is_version_supported`
jbesraa Feb 1, 2024
61a721b
Try v1 request if v2 fail in payjoin-cli
jbesraa Jan 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions payjoin-cli/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,22 @@ impl App {
Ok(())
}

#[cfg(feature = "v2")]
async fn try_v1(&self, req_ctx: &mut payjoin::send::RequestContext) -> Result<Psbt> {

@DanGould DanGould Feb 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: this file both sends and receives so this fn should probably be called try_send_v1

let (req, ctx) = req_ctx.clone().extract_v1().unwrap();
let http = http_agent()?;
println!("Sending fallback v1 request to {} after v2 request attempt fail", &req.url);
let response = spawn_blocking(move || {
http.post(req.url.as_ref())
.set("Content-Type", "text/plain")
.send_bytes(&req.body)
.map_err(map_ureq_err)
})
.await??;
println!("Sent fallback v1 request");
Ok(ctx.process_response(&mut response.into_reader())?)
}

#[cfg(feature = "v2")]
async fn long_poll_post(&self, req_ctx: &mut payjoin::send::RequestContext) -> Result<Psbt> {
loop {
Expand All @@ -211,12 +227,18 @@ impl App {
println!("Sent fallback transaction");
match ctx.process_response(&mut response.into_reader()) {
Ok(Some(psbt)) => return Ok(psbt),
Ok(None) => std::thread::sleep(std::time::Duration::from_secs(5)),
Err(re) => {
println!("{}", re);
log::debug!("{:?}", re);
Ok(None) => {
std::thread::sleep(std::time::Duration::from_secs(5));
continue;
}
}
Err(error) => {
if error.is_version_supported(&1) {
return self.try_v1(req_ctx).await;
};
println!("{}", error);
log::debug!("{:?}", error);
Comment on lines +236 to +239

@DanGould DanGould Feb 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

returning early on line 236 here will skip printing the error if one is encountered. Rather, if an error is encountered there it should run on the same log statements. This will also require try_v1 returns Result<Psbt, ResponseError> instead of an anyhow::Error too. Then the last line of try_v1 can just be the process function and does not need to be wrapped in Ok()

Err(mut error) => {
    if error.is_version_supported(&1) {
        match self.try_v1(req_ctx).await {
            Ok(res) => return Ok(res),
            Err(e) => error = e,
        }
    };
    println!("{}", error);
    log::debug!("{:?}", error);
}

}
};
}
}

Expand Down
14 changes: 14 additions & 0 deletions payjoin/src/send/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,20 @@ impl ResponseError {
Err(_) => InternalValidationError::Parse.into(),
}
}

/// Check whether or not the payjoin receiver supports the supplied version.
///
/// This is useful when you support multiple versions of payjoin. For example, if you support
/// v1 and v2 (BIP78 and BIP77 respectively) and sent a v2 request that resulted in this error,
/// you could pass `1` to test whether or not the receiver might respond well to another
/// request using the older version.
pub fn is_version_supported(&self, v: &u64) -> bool {
if let Self::WellKnown(WellKnownError::VersionUnsupported(_, supported)) = self {
supported.contains(v)
} else {
false
}
}
}

impl std::error::Error for ResponseError {}
Expand Down