Skip to content

Conversation

@JPeer264
Copy link
Member

@JPeer264 JPeer264 commented Dec 10, 2025

(closes #18449)
(closes JS-1281)

Problem

As of now, the user has no chance to disallow the manipulation of fetch errors, as we overwrite the error. This can cause problems as seen in #18449.

Solution

This adds a new option for the SDK (please be very critical about that new option here, since fetch has no integration this has to be added as a init option). always is the default and acts the same as it is now, so it is acting as feature:

enhanceFetchErrorMessages: 'always' | 'report-only' | false`

To give the user full control of how the errors are done there are 3 settings:

always report-only false
manipulate the error message directly
send only the changed message to Sentry

Special attention to reviewers

When having report-only the generated logs locally differ from the ones in Sentry. I am not quite sure if that would cause any problems. This is the only question which I don't have the answer to yet

Alternative

In case the size increase is too much, we can also have a boolean that disables that (which is on by default)

@JPeer264 JPeer264 requested review from Lms24 and logaretm December 10, 2025 16:04
@linear
Copy link

linear bot commented Dec 10, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.91 kB +0.35% +85 B 🔺
@sentry/browser - with treeshaking flags 23.42 kB +0.45% +104 B 🔺
@sentry/browser (incl. Tracing) 41.72 kB +0.31% +127 B 🔺
@sentry/browser (incl. Tracing, Profiling) 46.31 kB +0.25% +112 B 🔺
@sentry/browser (incl. Tracing, Replay) 80.31 kB +0.16% +127 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 70 kB +0.13% +88 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 84.99 kB +0.15% +124 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 97.21 kB +0.12% +110 B 🔺
@sentry/browser (incl. Feedback) 41.63 kB +0.24% +97 B 🔺
@sentry/browser (incl. sendFeedback) 29.59 kB +0.29% +85 B 🔺
@sentry/browser (incl. FeedbackAsync) 34.61 kB +0.31% +105 B 🔺
@sentry/react 26.64 kB +0.38% +99 B 🔺
@sentry/react (incl. Tracing) 43.89 kB +0.22% +95 B 🔺
@sentry/vue 29.38 kB +0.34% +98 B 🔺
@sentry/vue (incl. Tracing) 43.51 kB +0.24% +101 B 🔺
@sentry/svelte 24.93 kB +0.38% +92 B 🔺
CDN Bundle 27.34 kB +0.32% +87 B 🔺
CDN Bundle (incl. Tracing) 42.34 kB +0.25% +103 B 🔺
CDN Bundle (incl. Tracing, Replay) 79.05 kB +0.13% +101 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 84.5 kB +0.1% +81 B 🔺
CDN Bundle - uncompressed 80.37 kB +0.39% +310 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 125.72 kB +0.25% +310 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 242.25 kB +0.13% +310 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 255.02 kB +0.13% +310 B 🔺
@sentry/nextjs (client) 46.11 kB +0.22% +101 B 🔺
@sentry/sveltekit (client) 42.08 kB +0.27% +113 B 🔺
@sentry/node-core 51.66 kB +0.09% +45 B 🔺
@sentry/node 161.55 kB +0.04% +51 B 🔺
@sentry/node - without tracing 93.09 kB +0.05% +42 B 🔺
@sentry/aws-serverless 108.6 kB +0.04% +43 B 🔺

View base workflow run

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

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 9,386 - 8,907 +5%
GET With Sentry 1,774 19% 1,760 +1%
GET With Sentry (error only) 6,277 67% 6,292 -0%
POST Baseline 1,224 - 1,230 -0%
POST With Sentry 600 49% 590 +2%
POST With Sentry (error only) 1,068 87% 1,081 -1%
MYSQL Baseline 3,393 - 3,375 +1%
MYSQL With Sentry 451 13% 459 -2%
MYSQL With Sentry (error only) 2,752 81% 2,718 +1%

View base workflow run

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Sounds reasonable to me! Let's make sure to document the new option, thanks!

@Lms24
Copy link
Member

Lms24 commented Dec 17, 2025

(I was thinking for a moment if it makes sense for us to only add the option to the globalHandlersIntegration but that's very hard to discover for users and we have to pay the size hit either way. So I'm fine with adding the top-level option)

JPeer264 and others added 8 commits December 18, 2025 10:56
…ance-messages-off/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…ance-messages-off/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…ance-messages-report-only/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…ance-messages-off/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…ance-messages-report-only/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…ance-messages-report-only/test.ts

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
@JPeer264 JPeer264 force-pushed the jp/enhance-fetch-error-message branch from 0350acc to 308c1f2 Compare December 18, 2025 09:57
Comment on lines +118 to +119
const enhanceOption = client?.getOptions().enhanceFetchErrorMessages ?? 'always';
const shouldEnhance = enhanceOption !== false;
Copy link

Choose a reason for hiding this comment

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

Bug: client?.getOptions() can be undefined, but the subsequent access to .enhanceFetchErrorMessages is not optional, causing a TypeError if no client is available.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

In packages/core/src/instrument/fetch.ts, the code attempts to access enhanceFetchErrorMessages on the result of client?.getOptions(). The getClient() function can return undefined, in which case client?.getOptions() also returns undefined. The subsequent property access is not using optional chaining (?.), leading to an attempt to read a property on undefined. This will throw a TypeError, causing the fetch instrumentation to crash whenever the SDK client is not available (e.g., before initialization or when disabled).

💡 Suggested Fix

Use optional chaining for the entire property access path to safely handle cases where the client or options are undefined. Change the line to const enhanceOption = client?.getOptions()?.enhanceFetchErrorMessages ?? 'always'; to prevent the TypeError.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/core/src/instrument/fetch.ts#L118-L119

Potential issue: In `packages/core/src/instrument/fetch.ts`, the code attempts to access
`enhanceFetchErrorMessages` on the result of `client?.getOptions()`. The `getClient()`
function can return `undefined`, in which case `client?.getOptions()` also returns
`undefined`. The subsequent property access is not using optional chaining (`?.`),
leading to an attempt to read a property on `undefined`. This will throw a `TypeError`,
causing the fetch instrumentation to crash whenever the SDK client is not available
(e.g., before initialization or when disabled).

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7704468

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

Successfully merging this pull request may close these issues.

Appending hostname to fetch error messages breaks is-network-error and p-retry packages

2 participants