Skip to content
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

Pass on multipart attachments without a content-type #3224

Closed
supervacuus opened this issue Mar 6, 2024 · 2 comments
Closed

Pass on multipart attachments without a content-type #3224

supervacuus opened this issue Mar 6, 2024 · 2 comments

Comments

@supervacuus
Copy link
Contributor

This relates to getsentry/sentry-native#955.

A summary of the above issue is:

  • attachments sent via the crashpad_handler (which sends them as part of a multipart request) cannot be previewed in the web-ui because they all have content-type application/octet-stream by default.
  • attachments sent as envelope items typically do not get a content-type header, in which case they will be MIME detected in the backend and only assigned application/octet-stream if no MIME type could be identified.
  • users would like to see the same behavior for attachments sent via the crashpad backend.

To achieve this in the Native SDK, we'd remove the content-type header from the multipart attachments sent via the crashpad_handler. This would reflect how we send attachment envelop items, which we also send without content type (this seems to be the default for other SDKs).

But to make this change in the Native SDK meaningful, we'd need Relay to pass on multipart attachments without content type if the SDK sent them this way (similar to how envelope attachments' content types are untouched). Right now, Relay assigns application/octet-stream to incoming multipart attachments without a content type.

Are there currently any components that rely on the automatic content type assignment in Relay, rather than deferring to the MIME detector in the Python code?

@olksdr
Copy link
Contributor

olksdr commented Mar 8, 2024

Hey @supervacuus

thanks for opening an issue and a PR. We will discuss this with the team and post the update here or on your PR.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 2 Mar 8, 2024
iker-barriocanal added a commit that referenced this issue Mar 25, 2024
This PR visualizes the change required to go forward with the approach
suggested here: #3224

I introduced that `set_payload()` interface asymmetry only to avoid
changing all the call sites for this PR.

---------

Co-authored-by: Iker Barriocanal <[email protected]>
@supervacuus
Copy link
Contributor Author

Since the associated PR has been merged and deployed and preliminary tests show that multipart attachments are correctly detected and thus can be previewed in the web UI, we can close this. Thanks for the support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants