Repository navigation
Remove url::Url types from public API #513
Description
Our dependency is twofold, in function arguments and in errors. We can more easily maintain stability of our public API if we internalize our url dependencies in both function arguments and in error variants.
Using url::Url in the public API may be a form of tech debt, in that we took it on to get the API out without full consideration at the time the dep was first introduced by Kixunil.
Don't' depend on url::Url in function arguments
Instead, take Url arguments as IntoUrl trait implementations in core typestate machine instead of url::Url.
We have to re-export url::Url, bind to it, and have downstream implementers manually parse URLs before payjoin will accept them as arguments to many functions that produce requests e.g. Sender::extract_v2.
Instead, an IntoUrl trait like reqwest over &'a str, &'a String, String that handles Url validation inside the typestate machine would abstract this burden from an end user and make one less error for them to handle since it'd be encapsulated in our variants resulting from those extract or setup functions.
related: #459
Don't depend on url in public error variants.
We have no explicit url::ParseError variants in our Error types because they're only internal variants, but if we were to make those variants public we would. We also make downstream users construct their own Urls to pass to our functions which makes them handle the ParseError on their own rather than encapsulating that logic and error handling in our extract_ functions or similar.
related: #403
Metadata
Metadata
Assignees
Labels
Type
Projects
- Status
Activity
nothingmuch commentedon Jan 28, 2025
reqwest's
IntoUrlis sealed, but what you're thinking of wouldn't be, right?DanGould commentedon Jan 28, 2025
It would be sealed and we'd implement it for &'a str, &'a String and String just like reqwest.
DanGould commentedon Jan 30, 2025
I realize now that
reqwestalso re-exportsurl::Url, which is nice for convenience. There are circumstances where you want to parse URLs early, as in payjoin-cli configuration, and having the preferred type available alongside IntoUrl, with IntoUrl trait implementations on&UrlandUrleven more ergonomic.Use IntoUrl for more ergonomic function signatures (#520)
DanGould commentedon Mar 20, 2025
This is somewhat covered by #520, we're not going to remove url::Url (immediately), but we are using
IntoUrlto make the signatures more ergonomic. Whether we want to depend on the url::ParseUrl variants in the public API is a question I haven't answered yet, but am leaning to NOT. Better if we encapsulate it in our own so that downstream won't get a break from the dependency being upgraded.arminsabouri commentedon Sep 8, 2025
From @DanGould "If we get rid of this [
impl IntoUrl for [url::]Url], then we can use string types everywhere"https://docs.rs/payjoin/latest/payjoin/trait.IntoUrl.html#implementors
Replace TryFrom<&Url> with PjParam::parse
6 remaining items
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
DanGould commentedon Oct 3, 2025
Noted un-completed by #1141
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
Return String instead of Url for endpoint accessors
DanGould commentedon Oct 3, 2025
closed by #1141