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

Source Hash Extension for Source Maps #85

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Apr 7, 2023

This proposes and specifies support for source hashes in Sentry. This allows looking up source maps by the content hash of the transpiled / minified file. It leverages recent additions to Chrome and Edge (Caller.getScriptHash).

Relates to #81

Rendered RFC

Comment on lines +107 to +109
also want to add them to the manifest. For this we could leverage the digest
header, with the small suboptimal situation that the HTTP digest spec uses
a base64 hash:
Copy link
Member

Choose a reason for hiding this comment

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

We should just embrace the reality that the headers have nothing to do with HTTP anymore (even though they originally were inspired by that).

This is all a bit bikesheddy, but we should strive for consistency here between the protocol, the sourcemap JSON, and these headers. We use a mix of different names and string/objects all over the place.
I believe we are currently limited by headers being a String->String mapping. But we could relax that as well if needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would honestly just move all this stuff out of headers and keep toplevel, but as it stands these are actually still http headers in the current implementation. It really just overlays the HTTP fetch.

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