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

Handle headers as string[] #14

Open
mydea opened this issue Mar 6, 2024 · 6 comments
Open

Handle headers as string[] #14

mydea opened this issue Mar 6, 2024 · 6 comments

Comments

@mydea
Copy link
Contributor

mydea commented Mar 6, 2024

See: getsentry/sentry-javascript#10936

It seems that Undici 6.7.0 has changed the signature of headers, this probably also needs to be addressed here I believe?

@dotansimha
Copy link

I also noticed that while upgrading a NodeJS version from 21.7.1 to 21.7.2. This upgrade also includes an undici version upgrade (built-in in Node): https://github.com/nodejs/node/releases/tag/v21.7.2

The actual error is:

TypeError: request.headers.push is not a function
    at processHeader (node:internal/deps/undici/undici:6451:25)
    at Request.addHeader (node:internal/deps/undici/undici:6380:9)
    at setHeadersOnRequest (file:///app/index.js:184989:12)
    at _onRequestCreate (file:///app/index.js:184893:9)
    at Channel.publish (node:diagnostics_channel:142:9)
    at new Request (node:internal/deps/undici/undici:6281:27)
    at [dispatch] (node:internal/deps/undici/undici:9060:25)
    at Intercept (node:internal/deps/undici/undici:8795:20)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5659:16)
    at Client.dispatch (node:internal/deps/undici/undici:5675:44)
    at [dispatch] (node:internal/deps/undici/undici:5906:32)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5652:33)
    at Pool.dispatch (node:internal/deps/undici/undici:5675:44)
    at [dispatch] (node:internal/deps/undici/undici:9488:27)
    at Intercept (node:internal/deps/undici/undici:8795:20)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5659:16)

@dotansimha
Copy link

@mydea @djMax any idea where the exact piece of code that needs to be adjusted? and how to keep it backward compatible? I can send a PR if that helps. Thanks!

@djMax
Copy link
Contributor

djMax commented Apr 7, 2024

So I know that this module is being "redone" in the core opentelemetry SDKs, but let me give a look and see if it's easy.

@djMax
Copy link
Contributor

djMax commented Apr 7, 2024

I don't even see anything in this instrumentation the SETS headers, so I don't see how this causes the problem (but still looking). Is there a simple repro I can try?

@djMax
Copy link
Contributor

djMax commented Apr 7, 2024

Ok, I found it. New version will publish any minute now that hopefully fixes it, please test and let me know.

@dotansimha
Copy link

1.2.0 seems to fix the issue, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants