sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Baggage header size from sentry causes 431 error for us

Open gotenxds opened this issue 1 year ago • 15 comments

Environment

SaaS (https://sentry.io/)

What are you trying to accomplish?

We are sending requests from the client to the server

How are you getting stuck?

Getting this huge repeating header image

Where in the product are you?

Unknown

Link

No response

DSN

No response

Version

No response

gotenxds avatar Apr 09 '24 06:04 gotenxds

Assigning to @getsentry/support for routing ⏲️

getsantry[bot] avatar Apr 09 '24 06:04 getsantry[bot]

Hi @gotenxds , in order to make sure this lands on the right team, could you please share which Sentry SDK and version you are using, and what your Sentry SDK setup is?

InterstellarStella avatar Apr 09 '24 08:04 InterstellarStella

hi @InterstellarStella Thanks for the quick reply, of course We are using "@sentry/react": "^7.105.0" Our setup looks like so

function beforeSend(event) {
  Object.assign(event, {
    // Add any additional event information here
  });

  return event;
}

const sentryConfig = {};

function prepareSentryConfig(history) {
  Object.assign(sentryConfig, {
    beforeSend,
    integrations: [
      Sentry.replayIntegration(),
      Sentry.reactRouterV5BrowserTracingIntegration({
        history,
        routes: routeDefs.flattenedRoutes,
        matchPath,
      }),
    ],
    replaysSessionSampleRate: 0.01,
    replaysOnErrorSampleRate: 1.0,
    ignoreErrors: ['ResizeObserver loop limit exceeded'],
  });
}

function initializeSentry(userData) {
  if (__PRODUCTION__ && !__IS_CI__) {
    Sentry.init({
      ...sentryConfig,
      dsn: frontendConfig.sentryDsn,
      environment: frontendConfig.environment,
      release: frontendConfig.release,
      tracesSampler: 0.4,
    });

    Sentry.setUser({ id: userData.sub, username: userData.username });
    Sentry.setTag('company_id', userData.company_id);
    Sentry.setTag('company_name', userData.company_name);
  }
}

gotenxds avatar Apr 09 '24 08:04 gotenxds

Thank you @gotenxds ! I will be transferring this issue to the correct repo for triage.

InterstellarStella avatar Apr 09 '24 08:04 InterstellarStella

Hi, when are you calling initializeSentry?

lforst avatar Apr 09 '24 08:04 lforst

Hi @lforst, it depends, we have a few entry points to the system, but generally on first page load

gotenxds avatar Apr 09 '24 11:04 gotenxds

This looks like a bug to me. Unfortunately I currently don't know how to reproduce. Would you mind sharing a reproduction example that lets us examine this behaviour?

lforst avatar Apr 09 '24 13:04 lforst

@lforst The project this is happening in is 7y old and very big at this point so Isolating this will not be easy on our part, I cant try but I doubt I can just replicate this by creating a new clean react project.

Also this does not happen on every request, it happens sometimes but frequent enough to cause us a lot of app crashes. Happened around 2000~ times in the last month out of 20k event sentry recorded.

Can you perhaps make an educated guess on what might be the issue or give me some pointer on where to debug sentry's code?

gotenxds avatar Apr 10 '24 07:04 gotenxds

So depending on whether this is an XHR request or a fetch request (assuming this is on the browser) the code is either in

  • XHR: https://github.com/getsentry/sentry-javascript/blob/15d6cb17909da3284a05d8da9356ef2ce0c3bc60/packages/browser-utils/src/browser/request.ts#L348-L352
  • Fetch: https://github.com/getsentry/sentry-javascript/blob/15d6cb17909da3284a05d8da9356ef2ce0c3bc60/packages/core/src/fetch.ts#L119

Our XHR code looks like it might potentially duplicate headers. Can you narrow down what kind of request it is? The browser dev tools should tell you.

lforst avatar Apr 10 '24 10:04 lforst

We use axios which uses XHR, the dev tools + sentry replay confirms that, I was never able to reproduce it myself, we only see it in sentry and our logs, I'l try and repro this

gotenxds avatar Apr 10 '24 11:04 gotenxds

@lforst Upon further inspection of our logs it seems the the baggage header is not the only one being inflated, the sentry trace shows a lot of non-duplicated values, I assume these are event ids?

I'm not sure why would that happen as this request if the first in the chain. image

gotenxds avatar Apr 10 '24 12:04 gotenxds

I can see that our code may produce long headers, but in theory it should not. Would you be able to share a reproduction example? I could potentially see this happening if one particular XHRequest got sent over and over again.

lforst avatar Apr 11 '24 13:04 lforst

@lforst I'm trying to reproduce this locally but no success yet, this happens quite a bit on production (around 50 times a day) and causes page crashes on our app as it does not know how to handle this issue

gotenxds avatar Apr 16 '24 10:04 gotenxds

@gotenxds if we cannot reproduce it, would you mind sharing a bit more under which circumstances it happens? OS, browser, which kind of request?

lforst avatar Apr 18 '24 08:04 lforst

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

getsantry[bot] avatar May 16 '24 07:05 getsantry[bot]

Hey there,

Not sure if this can be re-opened, but our app is experiencing the exact same issue. Each subsequent request made from our NextJS client includes a duplicated entry under the Baggage header. For example, we have an /account/settings page that fires off 3 requests during load:

request 1 Screenshot 2024-06-04 at 9 17 21 AM

request 2 Screenshot 2024-06-04 at 9 17 31 AM

request 3 Screenshot 2024-06-04 at 9 17 43 AM

this pattern continues until the one of the fetch calls recieves back a 431 "Request header fields too large" response, which usually results in app hanging until hard refresh.

Using "@sentry/nextjs": "^7.112.2" for our Next app.

Taking a look at the logic that @lforst mentioned in https://github.com/getsentry/sentry-javascript/issues/11500#issuecomment-2047136837, I suspect this is a bug with the construction of the sentry baggage headers, specifically either here or here.

In both cases, doing a deduplication of existing key-value pairs might resolve the issue:

function deduplicateHeaderValues(headerString) {
  // Split the string into an array of key-value pairs
  const pairsArray = headerString.split(',');

  // Create a Map to store unique key-value pairs
  const uniquePairs = new Map();

  // Iterate over the pairs array
  pairsArray.forEach(pair => {
    const [key, value] = pair.split('=');
    // Store the pair in the Map, overwriting any duplicate keys
    uniquePairs.set(key, value);
  });

  // Convert the Map back to a string of key-value pairs
  const deduplicatedString = Array.from(uniquePairs).map(pair => pair.join('=')).join(',');

  return deduplicatedString;
}

// Example usage:
const headerString = 'sentry-environment=vercel-production,sentry-release=44628bc0cc9159d525751a0da2c82fa54d01124a,sentry-public_key=ca2f84b9f6e4d93d9c70dd70f4ad9bbe,sentry-trace_id=559215e003254fe5abecdb9dd001cd2e,sentry-environment=vercel-production,sentry-release=44628bc0cc9159d525751a0da2c82fa54d01124a,sentry-public_key=ca2f84b9f6e4d93d9c70dd70f4ad9bbe,sentry-trace_id=559215e003254fe5abecdb9dd001cd2e,sentry-environment=vercel-production,sentry-release=44628bc0cc9159d525751a0da2c82fa54d01124a,sentry-public_key=ca2f84b9f6e4d93d9c70dd70f4ad9bbe,sentry-trace_id=559215e003254fe5abecdb9dd001cd2e';

const deduplicatedHeaderString = deduplicateHeaderValues(headerString);
console.log(deduplicatedHeaderString);

etnichols avatar Jun 04 '24 13:06 etnichols