fix(http): Align HTTP client error type and value - #3934
Open
buenaflor wants to merge 16 commits into
Open
Conversation
Under --obfuscate, DioException.runtimeType resolves to a minified name such as Qx, which becomes the issue title. Register a DioExceptionTypeIdentifier so the type stays readable without requiring the obfuscation map upload. packages/dart and packages/flutter already ship an identifier; dio was the only integration without one. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
A bad response renders as four lines of boilerplate: the meaning of the status code, a link to MDN and generic remediation advice. Replace it with the status code and route, which are what identify the failure. The request URL and status code are already on the event, and the untouched DioException stays on SentryException.throwable for beforeSend. Gated to the cases that carry no information of their own - badResponse and a null message - so timeouts keep the duration that elapsed and connection errors keep the host that failed. Skipped entirely when the user set DioException.stringBuilder or the global readableStringBuilder. Fixes GH-2914 Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
failedRequestTargets was matched against requestOptions.path, which is relative to baseUrl, so a host-based target like ['myapi.com'] never matched and the request was silently not captured. Match on the resolved URL instead, as FailedRequestClient does. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
SentryHttpClientError appeared in no ExceptionTypeIdentifier, so the issue title obfuscated in release builds just like dio's did. Its toString() also prefixed the message with 'Exception: ', which no other SDK emits. The value now reads exactly as sentry-javascript's does: 'HTTP Client Error with status code: 404'. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
SentrySupabaseClientError carried the same two problems as SentryHttpClientError: no ExceptionTypeIdentifier, so the issue title obfuscated, and an 'Exception: ' prefix on the value that no other SDK emits. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The previous format repeated the exception type, which Sentry already renders as the issue title prefix, and embedded the request path, making the value vary per path parameter. Use the same string the HTTP client spec, sentry-javascript and FailedRequestClient produce, so a Dio 502 and a package:http 502 read identically. The route still travels on the request interface. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
SentryRequest sanitizes its headers, SentryResponse never did, so a response's Set-Cookie, Authorization or X-API-Key was sent verbatim whenever sendDefaultPii was on. Route both HTTP client paths through HttpSanitizer. SentryResponse derives its cookies field from the headers map, so the cookie is now read before sanitizing to keep that field populated, which matches how sentry-java's OkHttp interceptor treats it. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The interceptor required a status-code match, and a DioException without a response has none, so timeouts, DNS errors and TLS failures produced no event at all. FailedRequestClient captures the equivalent failures on the package:http path. A missing status code now means there is nothing to filter on rather than an automatic reject. Cancellations stay excluded, being a deliberate user action, and failedRequestTargets is still honoured. Co-Authored-By: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…-error-type-and-value
DioEventProcessor rewrote the exception value only for bad responses, leaving timeouts and connection errors with Dio's own text. Those messages restate the timeout the developer configured and append generic remediation advice, and the embedded duration gave every timeout setting its own issue title. sentry-javascript never surfaces the underlying error's message either: httpClientIntegration uses the error only to recover a stack trace and always sets the value to its synthetic string. A timeout now reads "HTTP Client Error: connectionTimeout". A user-supplied stringBuilder still wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Two gaps against the HTTP client spec and sentry-javascript, closed across the package:http, dio and supabase integrations. Requests to the DSN were captured like any other, so a failing transport could try to report the failure over the connection that just failed. sentry-javascript guards this with isSentryRequestUrl inside _shouldCaptureResponse; the helper added here matches the DSN host the same way. The mechanism also left `handled` unset. It is now true: the SDK caught the failure and reported it rather than letting it reach the app. sentry-javascript sets false, which is deliberately not copied. SentryEnvelope.fromEvent turns handled == false into containsUnhandledException, which tells the native SDKs to end the session as crashed, and captureFailedRequests defaults to true here while httpClientIntegration is opt-in there. Copying that value would report a crashed session for every captured 5xx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Three buttons covering the paths this branch touches: a 404 bad response, a connection timeout and a DNS failure. The latter two were dropped entirely before the interceptor learned to capture failures that carry no status code. Each swallows the DioException so the interceptor's event is the only one sent, rather than a second one from an implicit capture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. FeaturesFlutter
Logs
Other
FixesHttp
Other
Enhancements
DependenciesDeps
Flutter
Internal ChangesDart
Logs
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v10-branch #3934 +/- ##
=============================================
Coverage ? 88.30%
=============================================
Files ? 346
Lines ? 12370
Branches ? 0
=============================================
Hits ? 10923
Misses ? 1447
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
A failure with no status code took its value straight from the enum name, so issue titles read "HTTP Client Error: connectionTimeout" -- a leaked identifier. They now read "connection timeout", "connection error", "bad certificate" and so on. Only the types whose enum name reads badly are mapped; a type Dio adds in a future minor release falls back to the name rather than breaking the build, which a switch would. The detail behind a connection error is not lost by keeping the title generic: DioErrorExtractor already chains DioException.error as a cause, so a DNS failure still carries its "SocketException: Failed host lookup" on the event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The DSN skip tested whether the URL contained the DSN host, so a self-hosted DSN on sentry.example.com also suppressed capture for not-sentry.example.com. Failures on such a host went unreported with nothing to indicate why. Parse the URL and compare hosts instead. sentry-javascript's isSentryRequestUrl has the same substring flaw; the cost of being stricter is a Uri.tryParse per captured failure, and a relative URL cannot address the DSN so it yields an empty host and is kept. Reported by Bugbot on #3934. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8cd78be. Configure here.
Putting the response on contexts.response also started shipping the response body, which had previously been built but only ever reached the beforeSend hint, and hints are never serialized. With sendDefaultPii on, every failed request would carry up to 150 KB of response body -- more than the option asks for, and more than FailedRequestClient or the other SDKs send. The event now gets status code, headers, cookies and body size. The body stays on the hint, so beforeSend can still read it. Reported by Bugbot on #3934. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
A request that threw reported mechanism.description as "HTTP Client Error with status code: null". The reason string was built before knowing whether it would be used: a thrown exception is kept as-is, so only the synthesized SentryHttpClientError ever needs the description. The exception value was never affected -- ??= short-circuits, so the thrown ClientException and its own message stay in the title. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
HTTP client errors from
package:http, Dio and Supabase now produce the same short, obfuscation-stable exception the other Sentry SDKs do.Issue titles
Dio dumped its own
toString()into the value — the status code's meaning, an MDN link and generic remediation advice — and its type didn't survive obfuscation:package:httpand Supabase were already concise but carried a strayException:prefix, and their types didn't survive obfuscation either:Connection-level failures — timeouts, DNS errors, bad certificates — produced no event at all in Dio, because there's no status code to match against. They're now captured, with the specific cause chained rather than in the title, so grouping stays per failure kind instead of splintering per hostname:
Event payload
contexts.response(Dio)beforeSendhint and stopped theresendDefaultPiiAuthorization,Cookieand friends filtered outmechanism.handledtruefailedRequestTargets(Dio)baseUrl-relative path, so a host target never matchedImportant
Issue titles and grouping change for anyone already capturing failed requests, and the newly captured connection failures add event volume. That's why this targets
v10-branch.Two deliberate divergences from sentry-javascript: it never captures connection-level failures at all, where Dio now does, matching our own
FailedRequestClient. And it setshandled: falsewhere we settrue—handled == falseends the native session as crashed, andcaptureFailedRequestsdefaults to on here while js's integration is opt-in, so copying that value would report a crashed session for every captured 5xx.💡 Motivation and Context
Refs #2914. Obfuscated builds reported these as
Qxwith a wall of Dio's own remediation text as the value, which is unsearchable and groups badly. The spec and every other SDK use a single short line.💚 How did you test it?
Unit tests in each affected package. Manually verified on an Android emulator with three new example buttons covering a 404, a connection timeout and a DNS failure.
📝 Checklist
sendDefaultPiiis enabled🔮 Next steps
main— neither affects grouping.