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

Review usage of crate::receive::Error #312

Open
spacebear21 opened this issue Jun 27, 2024 · 1 comment
Open

Review usage of crate::receive::Error #312

spacebear21 opened this issue Jun 27, 2024 · 1 comment

Comments

@spacebear21
Copy link
Collaborator

Follow-up from #277 (review)

Server errors are meant to be returned as a response, but are being returned in places that don't always make sense, e.g.:

return Err(Error::Server("Output substitution is disabled.".into()));

.map_err(|e| Error::Server(e.into()))?

.map(|res| Psbt::from_str(&res.psbt).map_err(|e| Error::Server(e.into())))

It would be good to review usage of crate::receive::Error and improve error nesting generally.

@DanGould DanGould modified the milestones: Payjoin V2 beta, payjoin-1.0 Jun 27, 2024
spacebear21 added a commit to spacebear21/rust-payjoin that referenced this issue Aug 22, 2024
spacebear21 added a commit to spacebear21/rust-payjoin that referenced this issue Aug 26, 2024
spacebear21 added a commit to spacebear21/rust-payjoin that referenced this issue Aug 28, 2024
spacebear21 added a commit to spacebear21/rust-payjoin that referenced this issue Sep 6, 2024
@DanGould
Copy link
Contributor

Rather than having v2 wrapping the v1 logic, might it make more sense to have some core PSBT mutation receive logic that gets wrapped by the network-specific details of either v2 and v1 protocols? The v1 module could handle encoding the errors for HTTP (as is done now, since Error::Server is ~a 5xx error and Error::BadRequest is 4xx), and the inner error could be made which don't make as much sense in the context of v2 where the client sender speak HTTP directly to the receiver, and inner typestate error could be made to be generic, but not over the irrelevant HTTP error abstraction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants