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

feat(server): Support chunked formdata for Crashpad #721

Merged
merged 4 commits into from
Aug 19, 2020

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Aug 18, 2020

Google Crashpad has a limitation on the length of values for custom attributes. This makes it impossible to send scope information and event payloads using the default Crashpad uploader. In the past, our SDKs had to disable automatic uploading, and implement their own logic, which comes with a set of disadvantages. In the future, Crashpad will support custom attachments on all platforms, but until that is available, we need a workaround on our end.

An upcoming version of Electron will send long crashpad annotations in chunks to work around this limitation. When a long value is detected, the annotation is duplicated with a postfix of __n, where n is the 1-based index of the chunk.

This PR joins these chunks, assuming they come in random order, and then parses JSON to extract the payload. An event id is not allowed in these payloads, as it cannot be parsed efficiently in the endpoint. Instead, Relay generates a random event id and always overrides the one in the payload.

@jan-auer jan-auer self-assigned this Aug 18, 2020
@github-actions
Copy link

github-actions bot commented Aug 18, 2020

Messages
📖 Opted out of changelogs due to #skip-changelog.

Generated by 🚫 dangerJS against 4adbabc

@jan-auer
Copy link
Member Author

event_id_from_formdata still cannot handle this, and it's not clear to me how to implement that efficiently yet. Without that, we are not able to extract the event ID in time to respond correctly to the client.

} else if entry.key() == "sentry[event_id]" {

@jan-auer
Copy link
Member Author

Since this is meant for Crashpad, it would be acceptable to define that the event ID should not be set when using this mechanism. Relay will assign one automatically and return it in the response, which can be picked up to render user feedback dialogs.

@HazAT do you think that would be a fair limitation?

@jan-auer jan-auer requested review from HazAT and untitaker August 19, 2020 07:45
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

lgtm though I will say that I feel like we're building too many sdk specific things into relay

@jan-auer jan-auer changed the title feat(server): Support chunked formdata for Electron feat(server): Support chunked formdata for Crashpad Aug 19, 2020
@jan-auer
Copy link
Member Author

I feel like we're building too many sdk specific things into relay

@untitaker I see your point, this is very borderline. I will update the PR description with more reasoning on this.

@jan-auer jan-auer marked this pull request as ready for review August 19, 2020 09:30
@jan-auer jan-auer requested a review from a team August 19, 2020 09:30
@jan-auer jan-auer merged commit 5a47f7d into master Aug 19, 2020
@jan-auer jan-auer deleted the feat/electron-chunked-formdata branch August 19, 2020 11:55
jan-auer added a commit that referenced this pull request Aug 20, 2020
* master:
  feat(server): Support chunked formdata for Crashpad (#721)
jan-auer added a commit that referenced this pull request Aug 26, 2020
* master:
  build(workspace): Move relay binary to workspace crate (#733)
  fix: Fix data schemas github action (#730)
  fix(docs): Incorrect init command in README (#732)
  fix(getting-started): Fix documentation links to other pages (#729)
  feat(server): Add initial minidump scrubbing (#682)
  fix: Remove jsonschema docs (#727)
  fix(docs): Fix a broken link
  test(server): Add unit tests to tracked future (#725)
  feat(server): Support chunked formdata for Crashpad (#721)
  fix(danger): Comment only on missing changelogs (#723)
  test: Require any python3 and support python3.8 (#722)
  fix: Re-add pii=maybe to span databags (#720)
  fix(schema): Type out span data and tags (#713)
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.

2 participants