fix(sourcemaps): Remove distinction between `Source` and `MinifiedSource` for `sourcemaps inject` by szokeasaurusrex · Pull Request #1958 · getsentry/sentry-cli

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

szokeasaurusrex · 2024-02-26T17:01:17Z

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

FixesGH-1955

loewenheim

Note that the split between Source and MinifiedSource predates debug id injection. We should make doubly sure that unifying them doesn't break any other functionality.

…eport (#1962) The distinction between "Scripts" and "Minified Scripts" is arbitrary. #1958 already eliminates the distinction for sourcemaps inject, and since the same code is also used to distinguish between files to output as "Scripts" and "Minified Scripts," merging #1958 already will cause the two sections to merge in the sourcemaps upload output. However, with #1958, the two sections are merged as "Minified Scripts." If this PR is applied along with #1958 (which is my intention), it will cause the two sections to be merged as "Scripts." However, this PR is also completely independent of #1958 – applied alone, it will have the same effect of merging the "Minified Scripts" and "Scripts" sections of the sourcemaps upload report output under "Scripts."

fix(sourcemaps): Inject debug IDs into all JS files
d9868ad

szokeasaurusrex requested review from lforst and loewenheim February 26, 2024 17:01

loewenheim reviewed Feb 26, 2024
View reviewed changes

Comment threadsrc/utils/sourcemaps.rs

szokeasaurusrex added 5 commits February 28, 2024 14:44

Correct some tests
020ec37

Fix sourcemaps upload test
f6a60eb

Comment to clarify MinifiedSource usage
05c26ed

Merge branch 'master' into szokeasaurusrex/inject-all-js
37859e2

Add newlines at end of files
5a88310

szokeasaurusrex requested a review from loewenheim March 1, 2024 14:51

szokeasaurusrex mentioned this pull request Mar 1, 2024
fix: Merge "Scripts" and "Minified Scripts" under sourcemaps upload report #1962
Merged

szokeasaurusrex added 2 commits March 4, 2024 10:34

Merge branch 'master' into szokeasaurusrex/inject-all-js
977388c

fix test
3e44d67

szokeasaurusrex enabled auto-merge (squash) March 4, 2024 09:39

szokeasaurusrex disabled auto-merge March 4, 2024 09:46

szokeasaurusrex changed the title ~~fix(sourcemaps): Inject debug IDs into all JS files~~fix(sourcemaps): Remove distinction between Source and MinifiedSource for sourcemaps injectMar 4, 2024

loewenheim approved these changes Mar 4, 2024
View reviewed changes

szokeasaurusrex merged commit 708f457 into masterMar 4, 2024

szokeasaurusrex deleted the szokeasaurusrex/inject-all-js branch March 4, 2024 10:24

Zozman mentioned this pull request Mar 4, 2024
Added --treat-source-as-minified option for sourcemaps inject command #1956
Closed

This was referenced Mar 19, 2024
False negatives in minified file detection cause files to silently be skipped during injection #1814
Closed
Improve detection of minified source files #1519
Closed