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

Undici 6.7.0 changes to request headers breaking Sentry code #10936

Closed
3 tasks done
jessezhang91 opened this issue Mar 5, 2024 · 5 comments
Closed
3 tasks done

Undici 6.7.0 changes to request headers breaking Sentry code #10936

jessezhang91 opened this issue Mar 5, 2024 · 5 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug

Comments

@jessezhang91
Copy link
Contributor

jessezhang91 commented Mar 5, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

7.105.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

I can't figure out how to reproduce it locally but the issue occurs immediately on our staging and production servers for some reason.

I think it's related to undici@6.7.0 changes:
nodejs/undici#2708
nodejs/undici#2879

and this line of code here:

const headerLines = request.headers.split('\r\n');

Since the headers can now be arrays and iterables, this might not be properly accounting for that when attempting to call .split().

Pinning to undici@6.6.2 does not exhibit this behavior.

Expected Result

No errors

Actual Result

TypeError: request.headers.split is not a function
  File "/var/app/node_modules/@sentry/node/cjs/integrations/undici/index.js", line 266, col 39, in setHeadersOnRequest
    const headerLines = request.headers.split('\r\n');
  File "/var/app/node_modules/@sentry/node/cjs/integrations/undici/index.js", line 177, col 7, in _onRequestCreate
    setHeadersOnRequest(request, sentryTraceHeader, sentryBaggageHeader);
  File "node:diagnostics_channel", line 142, col 9, in Channel.publish
  File "/var/app/node_modules/undici/lib/core/request.js", line 189, col 23, in new Request
    channels.create.publish({ request: this })
  File "/var/app/node_modules/undici/lib/dispatcher/client.js", line 294, col 21, in [dispatch]
    const request = new Request(origin, opts, handler)
  File "/var/app/node_modules/undici/lib/interceptor/redirect-interceptor.js", line 11, col 16, in Intercept
    return dispatch(opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 158, col 12, in [Intercepted Dispatch]
    return dispatch(opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 179, col 40, in Client.dispatch
    return this[kInterceptedDispatch](opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/pool-base.js", line 143, col 28, in [dispatch]
    } else if (!dispatcher.dispatch(opts, handler)) {
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 150, col 29, in [Intercepted Dispatch]
    return this[kDispatch](opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 179, col 40, in Pool.dispatch
    return this[kInterceptedDispatch](opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/agent.js", line 105, col 23, in [dispatch]
    return dispatcher.dispatch(opts, handler)
  File "/var/app/node_modules/undici/lib/interceptor/redirect-interceptor.js", line 11, col 16, in Intercept
    return dispatch(opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 158, col 12, in [Intercepted Dispatch]
    return dispatch(opts, handler)
  File "/var/app/node_modules/undici/lib/dispatcher/dispatcher-base.js", line 179, col 40, in Agent.dispatch
    return this[kInterceptedDispatch](opts, handler)
  File "node:internal/deps/undici/undici", line 10929, col 55, in <anonymous>
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Mar 5, 2024
@jessezhang91 jessezhang91 changed the title Undici changes to request headers breaking Sentry code Undici 6.7.0 changes to request headers breaking Sentry code Mar 5, 2024
@AbhiPrasad
Copy link
Member

Good catch @jessezhang91 - would you be open to opening a PR to fix this? Otherwise we have to backlog in the time being until someone else can get to it.

@jessezhang91
Copy link
Contributor Author

I traced down the change to here:
https://github.com/nodejs/undici/pull/2874/files#diff-7d30f7ef62c49f60ef5b01576ca8898402aa790a09c623b13f9e0ea090fba7e6R151

but it seems like the docs haven't been updated to reflect that change:
https://github.com/nodejs/undici/blob/main/docs/docs/api/DiagnosticsChannel.md

so I'm unsure if it's intentional? I'll double check there and apply the updated changes via a PR here when given confirmation.

@jessezhang91
Copy link
Contributor Author

jessezhang91 commented Mar 5, 2024

Here's my rough attempt at a PR: #10938

I don't have an easy way to test my changes beyond editing the node_modules files via patch-package but I did test that in my codebase and confirmed that it did work properly.

@mydea
Copy link
Member

mydea commented Mar 7, 2024

Thank you, we'll take a look at the PR in the next days! 🎉

@AbhiPrasad
Copy link
Member

Released https://github.com/getsentry/sentry-javascript/releases/tag/7.106.0 with this fix - thanks for contributing @jessezhang91!

evanpurkhiser added a commit to getsentry/chartcuterie that referenced this issue May 9, 2024
This should fix an issue with undici

```
TypeError: request.headers.split is not a function
    at setHeadersOnRequest (/usr/src/app/node_modules/@sentry/node/cjs/integrations/undici/index.js:248:39)
    at _onRequestCreate (/usr/src/app/node_modules/@sentry/node/cjs/integrations/undici/index.js:158:9)
    at Channel.publish (node:diagnostics_channel:143:9)
    at new Request (node:internal/deps/undici/undici:6295:27)
    at [dispatch] (node:internal/deps/undici/undici:9077:25)
    at Intercept (node:internal/deps/undici/undici:8812:20)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5673:16)
    at Client.dispatch (node:internal/deps/undici/undici:5689:44)
    at [dispatch] (node:internal/deps/undici/undici:5920:32)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5666:33)
```

See getsentry/sentry-javascript#10936
evanpurkhiser added a commit to getsentry/chartcuterie that referenced this issue May 9, 2024
This should fix an issue with undici

```
TypeError: request.headers.split is not a function
    at setHeadersOnRequest (/usr/src/app/node_modules/@sentry/node/cjs/integrations/undici/index.js:248:39)
    at _onRequestCreate (/usr/src/app/node_modules/@sentry/node/cjs/integrations/undici/index.js:158:9)
    at Channel.publish (node:diagnostics_channel:143:9)
    at new Request (node:internal/deps/undici/undici:6295:27)
    at [dispatch] (node:internal/deps/undici/undici:9077:25)
    at Intercept (node:internal/deps/undici/undici:8812:20)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5673:16)
    at Client.dispatch (node:internal/deps/undici/undici:5689:44)
    at [dispatch] (node:internal/deps/undici/undici:5920:32)
    at [Intercepted Dispatch] (node:internal/deps/undici/undici:5666:33)
```

See getsentry/sentry-javascript#10936
outloudvi added a commit to outloudvi/info-pride that referenced this issue May 30, 2024
This should fix an issue tracked down to getsentry/sentry-javascript#10936 by upgrading Sentry SDK (we bumped to 8.x anyway).

Thank @Pain4Din0 for reporting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug
Projects
Archived in project
Development

No branches or pull requests

3 participants