Skip to content

Commit fd85225

Browse files
Andaristcodextimfish
authored
fix(bundler-plugins): Preserve directive prologues during bundle injection (#24221)
Bundler plugins inject Sentry code into generated bundles. In some outputs the injection could be placed before directive prologues such as `"use strict"`, causing JavaScript to stop recognizing them as directives. I found this issue to affect some production cases - I don't have access to their sources so I can't fully say how the original code was authored and what exactly made it lose the strict mode, but the generated output looked like this: ```js try{!function(){var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis? globalThis:"undefined"!=typeof self?self:{},t=(new e.Error).stack;t&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[t]="612bd636-5fcc-473a-bdd0-20460245872c",e._sentryDebugIdIdentifier="sentry-dbid-612bd636-5fcc-473a-bdd0- 20460245872c")}()}catch(e){}"use strict";(self.webpackChunk_N_E=self.webpackChunk_N_E||[]).push([[120],{67232:function(e,t,n){var r,l=n(41498),a=n(90413),o={usingClientEntryPoint:!1,Events:null,Dispatcher:{current:null}};function i(e){var t="https://react.dev/errors/"+e;if(1<arguments.length){t+="?args[]="+encodeURIComponent(arguments[1]); ``` Given the sentry code was injected before the strict mode directive, that changed the meaning of `arguments[1]` at this position in the app code: ```js function aW(e, t) { if (null !== (t = null !== (t = t.updateQueue) ? t.lastEffect : null)) { var n = (t = t.next); do { if ((n.tag & e) === e) { var r = n.create, l = n.inst; /* arguments[1] no longer refer to the original argument */ r = r(); l.destroy = r; } n = n.next; } while (n !== t); } } ``` That's because in the sloppy mode the assignment to `t` before the `arguments[1]` reference changes the `arguments` content too 🫠 . You can test it out using this isolated sample: ```js function test(foo) { foo = 2; console.log(arguments[0]); // 2 } test(1); function testStrict(foo) { "use strict"; foo = 2; console.log(arguments[0]); // 1 } testStrict(1); ``` This PR: - adds a bunch of tests for edge cases and for source mapping behavior (the latter was already working OK but didn't quite have the coverage) - replaces simple regex with a more spec-compliant tiny scanner so the proper injection point can be found - in Webpack `BannePlugin` can only prepend/append text, as far as I know, it can't just inject into an arbitrary position. So it was replaced with a compilation hook and ReplaceSource plugin. That allows for a fine-grained control at the asset level - in the case of Rollup, this PR only slightly changes the insertion point calculation - but it doesn't replace the overall mechanism/hooks used - esbuild has not required any fixes because `inject` API handles this for us --- AI disclosure: I have steered it a bunch myself and I understand each line of code added. I ensured (using my own judgement) that all of this matches the project's style and goal but ofc I have much less context on that than the maintainers here. That said, I can address any PR feedback thrown my way. --------- Co-authored-by: OpenAI Codex <codex@openai.com> Co-authored-by: Tim Fish <tim@timfish.uk>
1 parent 2bff2ea commit fd85225

36 files changed

Lines changed: 739 additions & 311 deletions
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import * as esbuild from "esbuild";
2+
import { sentryEsbuildPlugin } from "@sentry/bundler-plugins/esbuild";
3+
4+
await esbuild.build({
5+
entryPoints: {
6+
sloppy: "./src/sloppy-mode.cjs",
7+
},
8+
bundle: true,
9+
outdir: "./out/cjs-directives/without-plugin",
10+
outExtension: { ".js": ".cjs" },
11+
minify: false,
12+
format: "cjs",
13+
tsconfigRaw: { compilerOptions: { alwaysStrict: false } },
14+
});
15+
16+
await esbuild.build({
17+
entryPoints: {
18+
strict: "./src/strict-mode.cjs",
19+
sloppy: "./src/sloppy-mode.cjs",
20+
},
21+
bundle: true,
22+
outdir: "./out/cjs-directives/static-injection",
23+
outExtension: { ".js": ".cjs" },
24+
minify: false,
25+
format: "cjs",
26+
tsconfigRaw: { compilerOptions: { alwaysStrict: false } },
27+
plugins: [
28+
sentryEsbuildPlugin({
29+
telemetry: false,
30+
release: { name: "strict-mode-release", create: false },
31+
sourcemaps: { disable: true },
32+
}),
33+
],
34+
});
35+
36+
await esbuild.build({
37+
entryPoints: {
38+
strict: "./src/strict-mode.cjs",
39+
sloppy: "./src/sloppy-mode.cjs",
40+
},
41+
bundle: true,
42+
outdir: "./out/cjs-directives/debug-id-injection",
43+
outExtension: { ".js": ".cjs" },
44+
minify: false,
45+
format: "cjs",
46+
sourcemap: true,
47+
tsconfigRaw: { compilerOptions: { alwaysStrict: false } },
48+
plugins: [
49+
sentryEsbuildPlugin({
50+
telemetry: false,
51+
release: { inject: false },
52+
sourcemaps: { disable: "disable-upload" },
53+
}),
54+
],
55+
});
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { expect } from "vitest";
2+
import { test } from "./utils";
3+
4+
test(import.meta.url, ({ runBundler, runFileInNode }) => {
5+
runBundler();
6+
7+
expect(JSON.parse(runFileInNode("without-plugin/sloppy.cjs"))).toEqual({
8+
sloppyModePreserved: true,
9+
releaseInjected: false,
10+
debugIdInjected: false,
11+
});
12+
expect(JSON.parse(runFileInNode("static-injection/strict.cjs"))).toEqual({
13+
strictModePreserved: true,
14+
releaseInjected: true,
15+
debugIdInjected: false,
16+
});
17+
expect(JSON.parse(runFileInNode("static-injection/sloppy.cjs"))).toEqual({
18+
sloppyModePreserved: true,
19+
releaseInjected: true,
20+
debugIdInjected: false,
21+
});
22+
expect(JSON.parse(runFileInNode("debug-id-injection/strict.cjs"))).toEqual({
23+
strictModePreserved: true,
24+
releaseInjected: false,
25+
debugIdInjected: true,
26+
});
27+
expect(JSON.parse(runFileInNode("debug-id-injection/sloppy.cjs"))).toEqual({
28+
sloppyModePreserved: true,
29+
releaseInjected: false,
30+
debugIdInjected: true,
31+
});
32+
});
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
globalThis.sloppyModePreserved =
2+
(function () {
3+
return this;
4+
})() === globalThis;
5+
6+
console.log(
7+
JSON.stringify({
8+
sloppyModePreserved: globalThis.sloppyModePreserved,
9+
releaseInjected: globalThis.SENTRY_RELEASE?.id === "strict-mode-release",
10+
debugIdInjected: Object.keys(globalThis._sentryDebugIds || {}).length === 1,
11+
})
12+
);
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
"use strict";
2+
3+
globalThis.strictModePreserved =
4+
(function () {
5+
return this;
6+
})() === undefined;
7+
8+
console.log(
9+
JSON.stringify({
10+
strictModePreserved: globalThis.strictModePreserved,
11+
releaseInjected: globalThis.SENTRY_RELEASE?.id === "strict-mode-release",
12+
debugIdInjected: Object.keys(globalThis._sentryDebugIds || {}).length === 1,
13+
})
14+
);

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/after-upload-deletion.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/application-key.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};e._sentryModuleMetadata=e._sentryModuleMetadata||{},e._sentryModuleMetadata[(new e.Error).stack]=function(e){for(var n=1;n<arguments.length;n++){var a=arguments[n];if(null!=a)for(var t in a)a.hasOwnProperty(t)&&(e[t]=a[t])}return e}({},e._sentryModuleMetadata[(new e.Error).stack],{"_sentryBundlerPluginAppKey:1234567890abcdef":true});var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};e._sentryModuleMetadata=e._sentryModuleMetadata||{},e._sentryModuleMetadata[(new e.Error).stack]=function(e){for(var n=1;n<arguments.length;n++){var a=arguments[n];if(null!=a)for(var t in a)a.hasOwnProperty(t)&&(e[t]=a[t])}return e}({},e._sentryModuleMetadata[(new e.Error).stack],{"_sentryBundlerPluginAppKey:1234567890abcdef":true});var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/basic-cjs.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/basic-release-disabled.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/basic-sourcemaps.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");
1312
1413
/******/ })()
1514
;
1615
//# sourceMappingURL=basic.js.map",
17-
"basic.js.map": "{"version":3,"file":"basic.js","mappings":";;;AAAA;AACA","sources":["webpack://webpack5-integration-tests/./src/basic.js"],"sourcesContent":["// eslint-disable-next-line no-console\\nconsole.log(\\"hello world\\");\\n"],"names":[],"sourceRoot":""}",
16+
"basic.js.map": "{"version":3,"file":"basic.js","mappings":";;AAAA;AACA","sources":["webpack://webpack5-integration-tests/./src/basic.js"],"sourcesContent":["// eslint-disable-next-line no-console\\nconsole.log(\\"hello world\\");\\n"],"names":[],"sourceRoot":""}",
1817
"sentry-cli-mock.json": "["release","create","CURRENT_SHA","--project","fake-project"],
1918
["release","set-commits","CURRENT_SHA","--auto"],
2019
["release","finalize","CURRENT_SHA"],

dev-packages/bundler-plugin-integration-tests/fixtures/webpack5/basic.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
55
runBundler();
66
expect(readOutputFiles()).toMatchInlineSnapshot(`
77
{
8-
"basic.js": "!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();
9-
/******/ (() => { // webpackBootstrap
8+
"basic.js": "/******/ !function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?globalThis:"undefined"!=typeof self?self:{};e.SENTRY_RELEASE={id:"CURRENT_SHA"};var n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="00000000-0000-0000-0000-000000000000",e._sentryDebugIdIdentifier="sentry-dbid-00000000-0000-0000-0000-000000000000");}catch(e){}}();(() => { // webpackBootstrap
109
/******/ "use strict";
1110
// eslint-disable-next-line no-console
1211
console.log("hello world");

0 commit comments

Comments
 (0)