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(js): Parse debug ids from scraped source files #1534

Merged

Conversation

timfish
Copy link
Contributor

@timfish timfish commented Oct 14, 2024

I've been working on some bundler plugins that inject debug ids as per the TC39 proposal.

Since there's no cheap way to get debug ids from source files in the browser, this PR adds parsing of debug id from source files when they're scraped which can later be used to lookup the sourcemap.

The JavaScript SDKs can't rely on source files being publicly available and/or scraping working so we will still have to use polyfills and pass debug ids from the client. So this change doesn't improve things from the SDK perspective (no reduction in bundle or payload size), but I though symbolicator should probably still support this.

Let me know if this is worth adding and I'll try and add some tests!

@timfish timfish marked this pull request as ready for review October 14, 2024 23:44
@loewenheim
Copy link
Contributor

Thank you very much for the contribution, I think this is a good idea and seems like a simple change. Tests would be appreciated!

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.95%. Comparing base (5a85fd7) to head (614eecf).
Report is 158 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1534      +/-   ##
==========================================
- Coverage   76.09%   73.95%   -2.15%     
==========================================
  Files         101      104       +3     
  Lines       15262    15870     +608     
==========================================
+ Hits        11613    11736     +123     
- Misses       3649     4134     +485     

@loewenheim
Copy link
Contributor

Do you have a test case demonstrating the improvement that you can add?

@timfish
Copy link
Contributor Author

timfish commented Oct 16, 2024

I will add a test, I've just not had a chance yet!

@timfish
Copy link
Contributor Author

timfish commented Oct 17, 2024

Test added!

It has a source file which isn't included in the artifact bundle which causes it to be scraped. Once scraped, the debug id parsed and then used to lookup the sourcemap from the bundle.

@loewenheim
Copy link
Contributor

Very nice! Would you mind adding a changelog entry?

@loewenheim loewenheim self-assigned this Oct 17, 2024
@loewenheim loewenheim merged commit 6f6cf85 into getsentry:master Oct 21, 2024
13 checks passed
@timfish timfish deleted the feat/parse-debug-ids-from-scraped-source branch October 21, 2024 08:04
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