Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(node): Do not drop 300 and 304 status codes by default #17686

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

Merged
mydea merged 5 commits into develop from fn/no-304-filter
Sep 17, 2025

Conversation

@mydea
Copy link
Member

@mydea mydea commented Sep 17, 2025
edited
Loading

Looking at https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#redirection_messages, it seems that we should not filter out 300, 302 and 304 status codes by default:

300 Multiple Choices
In agent-driven content negotiation, the request has more than one possible response and the user agent or user should choose one of them. There is no standardized way for clients to automatically choose one of the responses, so this is rarely used.
304 Not Modified
This is used for caching purposes. It tells the client that the response has not been modified, so the client can continue to use the same cached version of the response.

In contrast, the others seem safe to continue to filter:

301 Moved Permanently
The URL of the requested resource has been changed permanently. The new URL is given in the response.

302 Found
This response code means that the URI of requested resource has been changed temporarily. Further changes in the URI might be made in the future, so the same URI should be used by the client in future requests.

303 See Other
The server sent this response to direct the client to get the requested resource at another URI with a GET request.

305 Use Proxy Deprecated
Defined in a previous version of the HTTP specification to indicate that a requested response must be accessed by a proxy. It has been deprecated due to security concerns regarding in-band configuration of a proxy.

306 unused
This response code is no longer used; but is reserved. It was used in a previous version of the HTTP/1.1 specification.

307 Temporary Redirect
The server sends this response to direct the client to get the requested resource at another URI with the same method that was used in the prior request. This has the same semantics as the 302 Found response code, with the exception that the user agent must not change the HTTP method used: if a POST was used in the first request, a POST must be used in the redirected request.

308 Permanent Redirect
This means that the resource is now permanently located at another URI, specified by the Location response header. This has the same semantics as the 301 Moved Permanently HTTP response code, with the exception that the user agent must not change the HTTP method used: if a POST was used in the first request, a POST must be used in the second request.

eventually it makes sense to unify this between node/node-core, but this should happen when we merge the httpIntegration split PR.

seer-by-sentry[bot] reacted with hooray emoji
@mydea mydea self-assigned this Sep 17, 2025
/**
* Do not capture spans for incoming HTTP requests with the given status codes.
* By default, spans with 404 status code are ignored.
* By default, spans with some 3xx and 4xx status codes are ignored.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super-l: Just to clarify?

Suggested change
* By default, spans with some 3xx and 4xx status codes are ignored.
* By default, spans with some 3xx and 4xx status codes are ignored(seebelow).

mydea reacted with thumbs up emoji
Copy link
Contributor

github-actions bot commented Sep 17, 2025
edited
Loading

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.16 kB - -
@sentry/browser - with treeshaking flags 22.72 kB - -
@sentry/browser (incl. Tracing) 40.16 kB - -
@sentry/browser (incl. Tracing, Replay) 78.53 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 68.24 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 83.19 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 95.4 kB - -
@sentry/browser (incl. Feedback) 40.87 kB - -
@sentry/browser (incl. sendFeedback) 28.81 kB - -
@sentry/browser (incl. FeedbackAsync) 33.75 kB - -
@sentry/react 25.88 kB - -
@sentry/react (incl. Tracing) 42.17 kB - -
@sentry/vue 28.64 kB - -
@sentry/vue (incl. Tracing) 41.98 kB - -
@sentry/svelte 24.19 kB - -
CDN Bundle 25.74 kB - -
CDN Bundle (incl. Tracing) 40.05 kB - -
CDN Bundle (incl. Tracing, Replay) 76.27 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 81.78 kB - -
CDN Bundle - uncompressed 75.18 kB - -
CDN Bundle (incl. Tracing) - uncompressed 118.5 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 233.6 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 246.37 kB - -
@sentry/nextjs (client) 44.15 kB - -
@sentry/sveltekit (client) 40.6 kB - -
@sentry/node-core 49.89 kB +0.07% +30 B 🔺
@sentry/node 151.23 kB +0.06% +78 B 🔺
@sentry/node - without tracing 91.81 kB +0.07% +58 B 🔺
@sentry/aws-serverless 105.26 kB +0.07% +67 B 🔺

View base workflow run

Copy link
Contributor

github-actions bot commented Sep 17, 2025
edited
Loading

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 8,635 - 8,818 -2%
GET With Sentry 1,288 15% 1,431 -10%
GET With Sentry (error only) 5,913 68% 6,176 -4%
POST Baseline 1,183 - 1,203 -2%
POST With Sentry 504 43% 530 -5%
POST With Sentry (error only) 1,030 87% 1,056 -2%
MYSQL Baseline 3,242 - 3,353 -3%
MYSQL With Sentry 373 12% 499 -25%
MYSQL With Sentry (error only) 2,613 81% 2,711 -4%

View base workflow run

@mydea mydea changed the title (削除) feat(node): Do not drop 300, 302 and 304 status codes by default (削除ここまで) (追記) feat(node): Do not drop 300 and 304 status codes by default (追記ここまで) Sep 17, 2025
@mydea mydea merged commit 2cde2a4 into develop Sep 17, 2025
187 of 188 checks passed
@mydea mydea deleted the fn/no-304-filter branch September 17, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lms24 Lms24 Lms24 approved these changes

@andreiborza andreiborza Awaiting requested review from andreiborza

@RulaKhaled RulaKhaled Awaiting requested review from RulaKhaled

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

AltStyle によって変換されたページ (->オリジナル) /