-
-
Notifications
You must be signed in to change notification settings - Fork 235
fix(sourcemaps): Fix double association bug #2764
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
Conversation
a1ca2bd
to
7e16121
Compare
Fixes a bug where the same sourcemap can be associated with mulitple source files. Fixes #2445 Fixes [CLI-5](https://linear.app/getsentry/issue/CLI-5/ensure-collect-sourcemap-references-does-not-associate-the-same)
7e16121
to
6fc0961
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The high level idea and implementation look good to me after carefully reading through the code.
I would suggest simplifying the code to prioritize ease of understanding. It's easy to lose track when there's a lot of nesting like this.
Maybe some of the processing logic can be extracted into functions.
If this is not possible/beneficial, we could at least add comments to the for_each
/fold
/filter
steps where the body is multiple lines, to make it easier to reason about specific steps in isolation.
Converting to draft while I work on the simplification, per @lcian's request |
@lcian is this good now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and much easier to understand, thanks!
Fixes a bug where the same sourcemap can be associated with mulitple source files.
Change description
We solve this problem by keeping track of the sourcemaps we have already associated with a source file. If we guess a sourcemap which has already been associated, we do not associate the given source file with any sourcemap.
Also, we iterate first through all the sources which explicitly reference the sourcemap – i.e., for which, we don't have to guess the sourcemap. These are associated first. As they are explicitly linked to a sourcemap, we intentionally do not prevent duplicates here.
On the second iteration, we guess the sourcemap reference for all remaining sources. We enforce the no duplicates rule on the second iteration. Sources cannot be associated with a sourcemap that was explicitly referenced by another source on the first iteration. If multiple sources guess the same sourcemap, and that sourcemap is not explicitly associated with any source, then none of the sources will be associated with that sourcemap (as it is unclear how that sourcemap should be associated).
In all cases, we warn the users that we failed to associate the sources properly.
Issues
Fixes #2445
Fixes CLI-5