fix: support response.url in net.fetch #44725
Conversation
| Subject: feat(undici): support urlList initialization in Response ctor | ||
|
|
||
| Extends the Response constructor to initialize with urlList array, | ||
| which will be used to support the url getter. |
There was a problem hiding this comment.
@codebytere I don't see a way to upstream this change, Response#init options does not support urlList. But we have a unique case of constructing the url list from a different http client (chromium) via undici. Any ideas ?
There was a problem hiding this comment.
@deepak1556 I think we could raise an issue or PR and solicit some feedback from undici - maybe they'll be able to support a more generalizable solution that addresses our use case here?
|
Just read the Lines 100 to 101 in 0005ae9 |
|
It would be great to get this fixed -- we have been bitten by this value missing a few times now. Since the typescript type for |
This comment was marked as outdated.
This comment was marked as outdated.
ff7dbd8 to
cc8aefb
Compare
There was a problem hiding this comment.
The general approach here looks good to me - from an API perspective i'd mostly just want to initiate conversation upstream alongside this to prevent potential future API breakage if they decide to support something slightly different based on our initial upstream effort!
| The patch adds the url chain to the URLResponseHead struct. This is | ||
| used to support the Response.url property in the Fetch API. | ||
|
|
||
| Should be upstreamed. |
There was a problem hiding this comment.
Not necessarily a blocker but I'd like to see some feedback from upstream as that potentially could result in subsequent changes to this PR if they want a slightly different API shape or approach or something!
| Subject: feat(undici): support urlList initialization in Response ctor | ||
|
|
||
| Extends the Response constructor to initialize with urlList array, | ||
| which will be used to support the url getter. |
There was a problem hiding this comment.
@deepak1556 I think we could raise an issue or PR and solicit some feedback from undici - maybe they'll be able to support a more generalizable solution that addresses our use case here?
Description of Change
Addresses
Response.url always returns '' (again, we'd have to patch undici to make this work)from #36733 (comment)This is useful is redirect scenarios where we currently miss the final url, also a prerequisite for #43715
Release Notes
Notes: fix url property in Response object returned by net.fetch