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

fix(bundler): some sourcemap generation bugs #11344

Merged
merged 27 commits into from
May 28, 2024
Merged

Conversation

paperdave
Copy link
Member

Before, if you bundled and minified console.log(React) you would get an invalid sourcemap

image

If you manually fixed the error causing this invalid data to be written, the rest of the sourcemap is off by ~375 bytes. Notice how nothing really aligns

image

Now:

image

Thanks to source map visualizer, vlq encoder/decoder, and mangs for helping me get a reproduction.

how did you verify your code works

no tests yet, all manual. i'd like to write some sourcemap tests, which will probably be snapshots to simplify the process.

Copy link
Contributor

github-actions bot commented May 25, 2024

@paperdave, your commit has failing tests :(

💻 3 failing tests Darwin x64 baseline

  • test/bundler/bundler_npm.test.ts 1 failing
  • test/js/node/tls/node-tls-context.test.ts 1 failing
  • test/js/web/workers/worker.test.ts 1 failing

💻 2 failing tests Darwin x64

  • test/bundler/bundler_npm.test.ts 1 failing
  • test/cli/install/bun-create.test.ts 1 failing

🐧💪 1 failing tests Linux AARCH64

  • test/bundler/bundler_npm.test.ts 1 failing

🐧🖥 1 failing tests Linux x64 baseline

  • test/bundler/bundler_npm.test.ts 1 failing

🐧🖥 1 failing tests Linux x64

  • test/bundler/bundler_npm.test.ts 1 failing

🪟💻 9 failing tests Windows x64 baseline

  • test/bundler/bundler_edgecase.test.ts 1 failing
  • test/cli/install/bunx.test.ts 1 failing
  • test/cli/install/migration/out-of-sync.test.ts 1 failing
  • test/cli/install/overrides.test.ts 1 failing
  • test/js/bun/shell/leak.test.ts 1 failing
  • test/js/bun/test/test-test.test.ts 1 failing
  • test/js/node/process/process.test.js 2 failing
  • test/js/node/tls/node-tls-context.test.ts code 1
  • test/js/node/watch/fs.watchFile.test.ts SIGKILL

🪟💻 5 failing tests Windows x64

  • test/bundler/bundler_edgecase.test.ts 1 failing
  • test/cli/install/bunx.test.ts 1 failing
  • test/js/node/process/process.test.js 2 failing
  • test/js/node/tls/node-tls-context.test.ts code 1
  • test/js/node/watch/fs.watchFile.test.ts 3 failing

View logs

Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

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

Serializing IRL discussion:

Snapshot-based sourcemap tests will be hard to debug when we inevitably change the output of the transpiler. They're certainly better than nothing though.

Also, Windows tests are failing

Overall looks great though

@mangs
Copy link
Contributor

mangs commented May 25, 2024

Great progress here! I love the screenshots (how did you make those arrows with the highlighted text?). A few updates from my testing:

  1. Using the sourcemap visualizer, I can now load the sourcemap for the entrypoint of our Bun lambda
  2. Unfortunately it looks like the sourcemap is not being used when errors are thrown. For example, I just ran it again with binary bun-11344 and got this output in the console:
1 | // @bun
2 | import{d as j} from"./index.8b306d478767a477.mjs";import"./index.04c119a3beb16a35.mjs";import{m as L,q as W,r as U,t as k} from"./index.2529db704c39a66b.mjs";var{dns:V}=globalThis.Bun;var{Glob:w}=globalThis.Bun;var A=["ALL","DELETE","GET","HEAD","OPTIONS","PATCH","POST","PUT"];class B{constructor(Q=!0){this.#X=[],this.#Y=Q}#X;#Y;async handleRequest(Q){const X=performance.now(),{method:Y}=Q,{pathname:Z}=new URL(Q.url);for(let[G,[z,$]]of this.#X){if(G!=="ALL"&&Y!==G)continue;if(new w(z).match(Z)){if(typeof $==="function"){if(this.#Y)Q.headers.append(...j("routeSync",X));return $(Q)}const[O]=Object.entries($);if(!O)throw new TypeError("No lazy-loaded configuration option provided");const[D,N]=O,C=(await N())[D];if(typeof C==="function"){if(this.#Y)Q.headers.append(...j("routeAsync",X));return C(Q)}throw new TypeError(`Lazy-loaded route handler target "${D}" was not a function`)}}throw new Error("No routes matched!")}#Q(Q,X,Y){if(!A.includes(Y))throw new TypeError(`"${Y}" is not a valid HTTP method`);return this. | ... truncated

error: oh noes
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:2:7535
      at f (/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:2:7828)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:201:20
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:48
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:100:29
      at measureElapsedTime (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:98:38)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:17
      at fetch (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:186:17)
warn: Could not decode sourcemap '/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs': InvalidSourceMap
  1. Notice the last line. Looks like there's a warning showing the sourcemap isn't being loaded for some reason.

Seems like we're getting close.

@mangs
Copy link
Contributor

mangs commented May 25, 2024

Here's the sourcemap from above with the sourcecode redacted. Notice sources has duplicate entries for the following files:

  • ../src/index.mts
  • ../node_modules/@mangs/bun-utils/src/routerUtils.mts
{
  "version": 3,
  "sources": ["../src/index.mts", "../node_modules/@mangs/bun-utils/src/routerUtils.mts", "../node_modules/@mangs/bun-utils/src/routerUtils.mts", "../src/types.mts", "../src/utils/awsSecretsManager.mts", "../src/index.mts"],
  "sourcesContent": [
    "<redacted>"
  ],
  "mappings": ";AACAACIAAC0HA,IAAS,UAA2B,CAAC,EAA0D,CAC7F,cACS,IAAa,UACpB,IAAa,QACX,WAAY,IAAY,MAAM,QAAQ,EAAS,MAAM,IACpD,iBAAkB,IAAY,MAAM,QAAQ,EAAS,YAAYAC5DxE,IAAS,UAAkC,CAAC,EAAsB,CAChE,MAAM,EAA6B,IAAI,IACvC,QAAW,KAAO,QAAQ,IACxB,GAAI,OAAO,OAAO,QAAQ,IAAK,CAAG,GAAK,EAAI,SAAS,CAAY,EAC9D,EAA2B,IAAI,EAAK,EAAS,QAAQ,IAAI,GAAO,CAAY,CAAC,EAGjF,OAAO,GAIT,eAAe,CAAiB,CAAC,EAA0C,EAAgB,CACzF,MAAQ,aAAc,KAAa,qCAC7B,EAA0B,IAAI,EAAU,CAC5C,YAAa,QAAQ,IAAI,kBACzB,SACA,gBAAiB,QAAQ,IAAI,sBAC7B,QAAS,iBACT,aAAc,QAAQ,IAAI,iBAC5B,CAAC,EAEK,EAAc,CAClB,aAAc,CAChB,EACM,EAAM,0BAA0B,mBAChC,EAAW,MAAM,EAAwB,MAAM,EAAK,CACxD,KAAM,KAAK,UAAU,CAAW,EAChC,QAAS,CACP,eAAgB,6BAChB,cAAc,IAAI,KAAK,GAAE,YAAY,EACrC,eAAgB,oCAClB,EACA,OAAQ,MACV,CAAC,EAED,IAAK,EAAS,GACZ,MAAM,IAAI,MAAM,oDAAqD,CACnE,MAAO,CACL,aAAc,MAAM,EAAS,KAAK,EAClC,WAAY,EAAS,OACrB,WAAY,EAAS,UACvB,CACF,CAAC,EAGH,MAAM,EAAiB,MAAM,EAAS,KAAK,EACrC,EAAe,EAA4B,CAAa,EAAI,EAAgB,CAAC,EACnF,GAAI,MAAM,QAAQ,EAAa,MAAM,GAAK,EAAa,OAAO,OAAS,EACrE,MAAM,IAAI,MAAM,sCAAuC,CAAE,MAAO,CAAE,cAAa,CAAE,CAAC,EAGpF,IAAK,MAAM,QAAQ,EAAa,YAAY,EAC1C,MAAM,IAAI,UAAU,0DAA2D,CAC7E,MAAO,CAAE,cAAa,CACxB,CAAC,EAGH,OAAO,EAAa,aAGtB,IAAS,UAAQ,CAAC,EAAa,EAAsB,CACnD,MAAM,EAAU,EAAI,MAAM,CAAQ,EAClC,IAAK,GAAS,OACZ,MAAM,IAAI,MAAM,oBAAqB,CAAE,MAAO,CAAE,KAAI,CAAE,CAAC,EAGzD,GAAI,EAAQ,OAAO,SAAW,EAC5B,MAAM,IAAI,MAAM,2BAA4B,CAC1C,MAAO,CACL,UAAW,EAAQ,OAAO,OAC1B,cACF,CACF,CAAC,EAGH,MAAO,CAAE,MAAK,OAAQ,EAAQ,MAAoC,GAGpE,eAAe,CAA6B,CAAC,EAAsB,CACjE,MAAM,EAAgC,EAAmC,CAAY,EACrF,GAAI,EAA8B,OAAS,EAAG,CAC5C,GAAI,QAAQ,IAAI,MACd,QAAQ,MAAM,4CAA4C,EAE5D,OAEF,GAAI,QAAQ,IAAI,MAAO,CACrB,MAAM,EAAS,EAA8B,OAAS,EACtD,QAAQ,MACN,GAAG,EAA8B,mCAAmC,EAAS,GAAK,YAClF,CACF,EAEF,MAAM,EAAe,MAAM,EACzB,CAAC,GAAG,EAA8B,KAAK,CAAC,EACxC,CACF,EAEA,QAAa,OAAM,kBAAkB,EAAc,CACjD,IAAK,IAAS,EACZ,SAEF,MAAM,EAAiB,EAA8B,IAAI,CAAI,EAC7D,GAAI,EAAgB,CAClB,MAAM,EAAoB,EAAK,QAAQ,IAAI,OAAO,GAAG,KAAe,EAAG,EAAE,GACjE,WAAY,EAAe,OAC7B,EAAc,EACf,KAAK,MAAM,CAAY,EAA6B,GACrD,EACJ,GAAI,KAAqB,QAAQ,IAC/B,MAAM,IAAI,MACR,8FACA,CACE,MAAO,CACL,KAAM,EACN,mBACF,CACF,CACF,EAEF,QAAQ,IAAI,GAAqB,IA7HvC,IAAM,EACJ,4JACI,EAAe",
  "debugId": "F659283F1B37485A64756e2164756e21",
  "names": []
}

@mangs
Copy link
Contributor

mangs commented May 25, 2024

After reading over the spec for source map version 3 and understanding more how this all works, it looks like the mappings field is truncated because the leading character is the group separator character ;.

As for tests, I found a good article about VLQ encoding/decoding that has a simple NPM package associated with it that might be helpful for writing test assertions for the mappings field. See article here and associated NPM package repo here (specifically was thinking base64VlqDecode could be useful, something Mozilla's source-map package doesn't expose).

@paperdave
Copy link
Member Author

paperdave commented May 25, 2024

the leading character can be a ;. this indicates there are no mappings on the first line. the invalid part of that sourcemap is this mapping "AACAACIAAC0HA" which looking at it by hand seems to be three mappings at once: "aaca", "acia", and "ac0ha"
its missing either commas or semis

so yes more to be done

@paperdave paperdave marked this pull request as draft May 25, 2024 19:11
@mangs
Copy link
Contributor

mangs commented May 26, 2024

As requested, the following is for the entry point code file and its accompanying source map:

1. No minify

  • Loads in source map visualizer: ✅
  • Uses source maps for errors: ❌
  • Shows warning that source maps could not load: ❌
  • Stack trace shown in the console:
210 | var secretSuffix = "_SECRET_ARN";
211 |
212 | // src/index.mts
213 | async function lambdaHandler(request) {
214 |   customizeRollbarPayload(request);
215 |   throw new Error("nope");
              ^
error: nope
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:215:9
      at lambdaHandler (/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:213:30)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:201:20
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:48
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:100:29
      at measureElapsedTime (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:98:38)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:17
      at fetch (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:186:17)

2. No splitting

  • Loads in source map visualizer: ✅
  • Uses source maps for errors: ❌
  • Shows warning that source maps could not load: ✅
  • Stack trace shown in the console:
37 |       ${X.map((z)=>`<Path>${z}</Path>`).join("\n      ")}
38 |     </Items>
39 |     <Quantity>${X.length}</Quantity>
40 |   </Paths>
41 | </InvalidationBatch>
42 | `.trim();try{const z=`https://cloudfront.amazonaws.com/2020-05-31/distribution/${Z}/invalidation`,H=await Y.fetch(z,{body:K,method:"POST"}),V=await H.text();if(H.ok)console.log("[INVALIDATE] CloudFront invalidation response:",V);else J0.error("Error response received in invalidateCloudfront()",{responseText:V,statusCode:H.status,statusText:H.statusText});return V}catch(z){if(J0.error("Unexpected error received in invalidateCloudfront()",z),z instanceof Error)return z.message;return"UNEXPECTED ERROR"}}var s9=V0(()=>{z1()});async function tY(X){const Z=new URL(i0);Z.searchParams.set("apiKey",process.env.BUILDER_API_KEY),Z.searchParams.set("staleCacheSeconds",T1.toString()),Z.searchParams.set("url",X),console.log(`[PRECACHE] Precaching path ${X} for ${rY.format(T1)} seconds`);try{const W=await fetch(Z),Y=await W.json();if(!W.ok)J0.error("Error response received in precacheBuilderContent()",{responseJson:Y,statusCode:W.status,statusText:W.statusText});return Y}catch(W){return J0.error("Unexpected error occurred i | ... truncated

error: nope
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:42:11796
      at XK (/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:42:12088)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:201:20
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:48
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:100:29
      at measureElapsedTime (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:98:38)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:17
      at fetch (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:186:17)
warn: Could not decode sourcemap '/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs': InvalidSourceMap

3. No minify or splitting

  • Loads in source map visualizer: ✅
  • Uses source maps for errors: ❌
  • Shows warning that source maps could not load: ❌
  • Stack trace shown in the console:
9284 | var secretSuffix = "_SECRET_ARN";
9285 |
9286 | // src/index.mts
9287 | async function lambdaHandler(request) {
9288 |   customizeRollbarPayload(request);
9289 |   throw new Error("nope");
               ^
error: nope
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:9289:9
      at lambdaHandler (/Users/egoldstein/code/babbel/get.apigateway.lambda/dist/index.mjs:9287:30)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:201:20
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:48
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:100:29
      at measureElapsedTime (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/timeUtils.mts:98:38)
      at /Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:190:17
      at fetch (/Users/egoldstein/code/babbel/get.apigateway.lambda/node_modules/@mangs/bun-utils/src/networkUtils.mts:186:17)

@paperdave
Copy link
Member Author

Thanks for the error samples, looks like there is more than one issue

the following combo:

Loads in source map visualizer: ✅
Uses source maps for errors: ❌
Shows warning that source maps could not load: ❌

is very worrysome as it means it generated a valid sourcemap, loaded it back into memory, but has corrupt mappings.

The plan is going to be to merge this PR with test fixes, and continue mangs' issue in another followup PR, repeating this process until their issues are fixed.

@paperdave paperdave changed the title fix(bundler): numerous sourcemap generation bugs fix(bundler): some sourcemap generation bugs May 28, 2024
@paperdave paperdave marked this pull request as ready for review May 28, 2024 22:46
@Jarred-Sumner Jarred-Sumner merged commit 96f29e8 into main May 28, 2024
46 of 53 checks passed
@Jarred-Sumner Jarred-Sumner deleted the dave/bundle-sourcemaps branch May 28, 2024 23:51
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.

9 participants