You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Unlike with got and request the response object from node-fetch doesn't treat the response headers as a standard object and the API requires a different access mechanism (e.g. resp.headers.get('header-name')) - https://github.com/node-fetch/node-fetch#class-headers; standard property access means do not work.
I ran into this in #7021 because the GitLab API-based badges are using the standard request fetcher while needing to inspect the response headers, unlike other response-header-inspecting badges which utilize a custom request fetcher (e.g. GitHub).
I can see two paths to resolve this, we either allow some more of the underlying http client specifics to bleed through the encapsulation layer to consuming service classes, or, map/convert the response headers within the encapsulation layer similar to how we're encapsulating the mapping/conversion of the http request parameters within the encapsulation layer as well as the returned object property names.
I definitely prefer the latter and will open a PR to that effect, but certainly open to discussion.
calebcartwright
changed the title
node-fetch response headers "missing" due API difference
node-fetch response headers "missing" due to API difference
Oct 17, 2021
Tbh, having deployed the node-fetch PR and kept an eye on the graphs for a week or so I think my assessment is that we've seen no meaningful change in memory usage at all and maybe a slight reduction in dyno load. I was going to leave it for a few days - maybe until the next ops meeting - but I think I'm pretty sold that the right next step is to revert #6914 and double down on implementing got more widely as the replacement for request (and finally get #4655 done), rather than carry on down the node-fetch path.
Unlike with
got
andrequest
the response object fromnode-fetch
doesn't treat the response headers as a standard object and the API requires a different access mechanism (e.g.resp.headers.get('header-name')
) - https://github.com/node-fetch/node-fetch#class-headers; standard property access means do not work.I ran into this in #7021 because the GitLab API-based badges are using the standard request fetcher while needing to inspect the response headers, unlike other response-header-inspecting badges which utilize a custom request fetcher (e.g. GitHub).
I can see two paths to resolve this, we either allow some more of the underlying http client specifics to bleed through the encapsulation layer to consuming service classes, or, map/convert the response headers within the encapsulation layer similar to how we're encapsulating the mapping/conversion of the http request parameters within the encapsulation layer as well as the returned object property names.
I definitely prefer the latter and will open a PR to that effect, but certainly open to discussion.
cc @chris48s
The text was updated successfully, but these errors were encountered: