Conversation
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 972382b. Configure here.
Add opt-in Crashpad handler support for Sentry large attachment uploads. Upload oversized minidumps and report attachments separately, then reference them from the final envelope. Expose HTTP response status and headers to the upload thread. Use that metadata to create uploads, stream file bytes, and submit the envelope. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Apply the existing empty Expect header to all non-GET request bodies so libcurl upload methods such as PATCH avoid waiting for 100 Continue responses. Co-Authored-By: OpenAI Codex <noreply@openai.com>
libcurl adds application/x-www-form-urlencoded for POST requests when no Content-Type is supplied. Clear the generated header so TUS creation requests can intentionally send no Content-Type. Move the local case-insensitive header comparison into the shared HTTP header helper so upload code and libcurl setup use the same behavior. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Populate response_headers() from every HTTP response header on the Mac and WinHTTP transports instead of only preserving Location. This keeps the transport API behavior consistent with libcurl and socket implementations for callers that read non-Location headers. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Build TUS upload and envelope URLs from the original minidump URL authority instead of reconstructing the origin from CrackURL's normalized port. This keeps default ports implicit unless the caller supplied them explicitly. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Let HTTPTransport callers override connect and transfer timeouts separately while preserving SetTimeout as the compatibility setter for both values. Disable the transfer timeout for TUS PATCH byte uploads so large attachments are not bounded by the normal report upload deadline. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Return kRetry when an eligible large attachment or minidump TUS upload fails instead of falling through to another upload format. This avoids mixing partially uploaded TUS refs with inline envelope or multipart fallback data for the same report. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Map a zero transfer timeout to a no-timeout NSURLRequest interval on Mac. This keeps large TUS uploads from inheriting the short connect timeout. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Continue with the standard report upload when a 404 disables the large attachment endpoint. Keep retrying transient TUS failures while the endpoint is still considered available. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Store the parsed socket transport status and headers even when reading the response body fails. This lets callers handle HTTP errors from chunked responses without implementing chunked decoding. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Use checked multiplication for the libcurl response header callback length. This keeps the callback consistent with the request and response body callbacks. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Return after headers for status codes that cannot carry response bodies. Also treat an explicit zero Content-Length as complete so keep-alive connections do not make the socket transport wait for EOF. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Use the connect timeout as the NSURLRequest idle timeout when callers disable the transfer timeout. This preserves macOS' only configured timeout guard while documenting that NSURLRequest does not expose separate connect and transfer timeouts. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Do not attach an HTTP body stream when callers explicitly set Content-Length to 0. CFNetwork can otherwise synthesize a form Content-Type for empty POST requests, which breaks TUS upload creation. Preserve response bodies before status validation so Mac callers can inspect server error responses. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Remove the multipart builder special case for Sentry attachment reference MIME types. Attachment references are emitted through envelopes, so no caller sends them through multipart uploads. Co-Authored-By: OpenAI Codex <noreply@openai.com>
972382b to
e0275c9
Compare
Route both envelope attachment input forms through the same named reader path. Keep event, breadcrumb, and regular attachment handling in one place. Do not change the public overloads. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Compare HTTP header names with string_view arguments. This avoids a C string dependency in the shared header and accepts existing string and literal call sites without constructing strings. Co-Authored-By: OpenAI Codex <noreply@openai.com>
| // kRetryWorkIntervalSeconds. | ||
| const int kRetryAttempts = 5; | ||
|
|
||
| constexpr uint64_t kSentryLargeAttachmentSize = 100ull * 1024 * 1024; // 100 MiB |
There was a problem hiding this comment.
We had 200MB on the old endpoint until now, would this change existing behavior?
There was a problem hiding this comment.
Good catch, but now I'm a bit confused... I probably got 100MB from the developer docs 🙄
Workflow:
If an attachment is larger than ~100 MiB, upload it to the TUS-conformant upload endpoint (requires feature
projects:relay-upload-endpoint):
https://develop.sentry.dev/sdk/telemetry/attachments/#attachment-placeholders
There was a problem hiding this comment.
Notice that the 100mb threshold only applies when the enable_large_attachments option is enabled. It would indeed change existing behavior with 100-200mb attachments, but only for those who opt in.
There was a problem hiding this comment.
Just had a chat with @jjbayer and @tobias-wilfert - 100MB it is - we accept the change in behavior for 100-200MB changes here, all good.
|
Let's hold this off - getsentry/relay#5877 is good enough for now, as it already supports large user attachments (logs etc.) out of the box. The only thing missing is support for large minidumps themselves, which is also already being worked on on the server side. This PR made use of the existing envelope conversion (originally made for external crash reporters), and wired it through the envelope endpoint to be able to use normal attachment-refs, but it adds so much extra noise in crashpad that it's much better done on the server side. |

Required for / tested by:
Examples with inflated ~128MB minidumps:
Also, downloads from Sentry have been verified to match:
$ sha256sum .sentry-native/completed/5988aa84-b912-4784-4d57-3e9cabc15227.dmp 5f173c73c9cc42b57f93694fc191c53721a1b9d681e4234df252f0df5a693977 $ sha256sum ~/Downloads/5988aa84-b912-4784-4d57-3e9cabc15227.dmp 5f173c73c9cc42b57f93694fc191c53721a1b9d681e4234df252f0df5a693977