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

Line numbers in browser stack trace are off by 2 for .js files #3371

Closed
Vages opened this issue Jan 16, 2022 · 3 comments · Fixed by #5644
Closed

Line numbers in browser stack trace are off by 2 for .js files #3371

Vages opened this issue Jan 16, 2022 · 3 comments · Fixed by #5644
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Milestone

Comments

@Vages
Copy link

Vages commented Jan 16, 2022

Describe the bug

When displaying stack traces in the browser, the line numbers are sometimes off by 2. See reproduction repo below.

capitalize-test: always off by 2 in browser

Try opening capitalize-test (probably http://localhost:3000/capitalize-test) in your browser. The stacktrace output in the browser will always be as follows (note the line 2 in /src/routes/_capitalize.js:2:22, while the line actually occurs on line 4 in the file):

Cannot read property 'toUpperCase' of undefined

TypeError: Cannot read property 'toUpperCase' of undefined
    at Module.capitalize (/src/routes/_capitalize.js:2:22)
    at capitalize-test.svelte:5:16
    at Object.$$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at Object.default (root.svelte:43:47)
    at eval (/.svelte-kit/runtime/components/layout.svelte:8:41)
    at Object.$$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at root.svelte:37:45
    at $$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at Object.render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1753:26)
    at render_response (file://<path-to-parent-folder>/offset-demo/node_modules/@sveltejs/kit/dist/chunks/index.js:651:28)

The output in the terminal from the dev server correctly displays line 4 in the stack trace:

Cannot read property 'toUpperCase' of undefined
TypeError: Cannot read property 'toUpperCase' of undefined
    at Module.capitalize (/src/routes/_capitalize.js:4:22)
    at capitalize-test.svelte:5:16
    at Object.$$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at Object.default (root.svelte:43:47)
    at eval (/.svelte-kit/runtime/components/layout.svelte:8:41)
    at Object.$$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at root.svelte:37:45
    at $$render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1745:22)
    at Object.render (<path-to-parent-folder>/offset-demo/node_modules/svelte/internal/index.js:1753:26)
    at render_response (file://<path-to-parent-folder>/offset-demo/node_modules/@sveltejs/kit/dist/chunks/index.js:651:28)

Try refreshing a couple of times. It should not make a difference to the server or browser output.

on-line-1-test:

off by 2 in browser on first load

Try opening on-line-1-test (probably http://localhost:3000/on-line-1-test) in your browser. The stacktrace output in the browser is as follows on the first try:

Line must be greater than or equal to 1, got -1

TypeError: Line must be greater than or equal to 1, got -1
    at BasicSourceMapConsumer.SourceMapConsumer_findMapping [as _findMapping] (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:58884:13)
    at BasicSourceMapConsumer.SourceMapConsumer_originalPositionFor [as originalPositionFor] (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:58953:22)
    at <path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59894:34
    at String.replace (<anonymous>)
    at <path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59882:21
    at Array.map (<anonymous>)
    at ssrRewriteStacktrace (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59881:10)
    at instantiateModule (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:60105:28)

The dev server gives roughly the same output.

TypeError: Cannot read property 'toUpperCase' of undefined
    at /src/routes/_error-on-line-1.js:1:11
    at instantiateModule (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:60102:15)
Line must be greater than or equal to 1, got -1
TypeError: Line must be greater than or equal to 1, got -1
    at BasicSourceMapConsumer.SourceMapConsumer_findMapping [as _findMapping] (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:58884:13)
    at BasicSourceMapConsumer.SourceMapConsumer_originalPositionFor [as originalPositionFor] (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:58953:22)
    at <path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59894:34
    at String.replace (<anonymous>)
    at <path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59882:21
    at Array.map (<anonymous>)
    at ssrRewriteStacktrace (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:59881:10)
    at instantiateModule (<path-to-parent-folder>/offset-demo/node_modules/vite/dist/node/chunks/dep-0351185a.js:60105:28)
Works if you refresh the page or open it in a new tab

If you refresh the page, you get an error message in the browser that looks correct to me (and no output from the dev server), but varies slightly by browser.

Firefox
undefined has no properties

@http://localhost:3000/src/routes/_error-on-line-1.js:1:1
Chrome
Cannot read properties of undefined (reading 'toUpperCase')
TypeError: Cannot read properties of undefined (reading 'toUpperCase')
    at http://localhost:3000/src/routes/_error-on-line-1.js:1:11
Safari
undefined is not an object (evaluating 'undefined.toUpperCase')
module code@http://localhost:3000/src/routes/_error-on-line-1.js:1:10
evaluate@[native code]
moduleEvaluation@[native code]
moduleEvaluation@[native code]
@[native code]
asyncFunctionResume@[native code]
@[native code]
promiseReactionJobWithoutPromise@[native code]
promiseReactionJob@[native code]

This applies to all supported node versions

I have tried this with node versions 14, 16 and 17. The behavior is exactly the same in all cases.

Reproduction

git clone https://github.com/Vages/offset-demo.git
cd offset-demo
npm install
npm run dev

Navigate to the routes capitalize-test and on-line-1-test, probably found at these URLs:

Logs

No response

System Info

System:
    OS: macOS 12.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 259.40 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.18.3 - ~/.asdf/installs/nodejs/14.18.3/bin/node
    Yarn: 1.22.4 - ~/.asdf/shims/yarn
    npm: 6.14.15 - ~/.asdf/plugins/nodejs/shims/npm
    Watchman: 2022.01.03.00 - /usr/local/bin/watchman
  Browsers:
    Chrome: 97.0.4692.71
    Firefox: 95.0.2
    Firefox Developer Edition: 70.0
    Safari: 15.2
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.10 
    @sveltejs/kit: next => 1.0.0-next.231 
    svelte: ^3.44.0 => 3.46.2

Severity

serious, but I can work around it

Additional Information

Possibly related issue

I was able to find this Vite PR (from Rich Harris, accidentally – or perhaps not), which compensates for stack trace offsets in node versions later than 13: vitejs/vite#2266 I thought there could possibly be something different about node versions 14 and later and tried running the following script (named offset.js) in node versions 12 through 16.

let offset
try {
  new Function('throw new Error(1)')()
} catch (e) {
  // in Node 12, stack traces account for the function wrapper.
  // in Node 13 and later, the function wrapper adds two lines,
  // which must be subtracted to generate a valid mapping
  console.log(e.stack);
  const match = /:(\d+):\d+\)$/.exec(e.stack.split('\n')[1])
  offset = match ? +match[1] - 1 : 0
}
console.log(offset);

The output was consistent from versions 14 and on:

Error: 1
    at eval (eval at <anonymous> (<path-to-parent-folder>/offset.js:3:3), <anonymous>:3:7)
    at Object.<anonymous> (<path-to-parent-folder>/offset.js:3:37)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47
2

If you put offset.js in a folder with a package.json containing the following,

{
  "type": "module"
}

... you get the following stack trace instead:

Error: 1
    at eval (eval at <anonymous> (file://<path-to-parent-folder>/offset.js:3:3), <anonymous>:3:7)
    at file://<path-to-parent-folder>/offset.js:3:37
    at ModuleJob.run (internal/modules/esm/module_job.js:183:25)
    at async Loader.import (internal/modules/esm/loader.js:178:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)
2
@Vages
Copy link
Author

Vages commented Jan 16, 2022

I have considered that Vite may be to blame in some way. I do not have more hours to spare for debugging this in the few coming days, so I'm posting this hoping for comments from someone who is more familiar with the Svelte Kit architecture than I am. Any feedback is welcome!

If no one else wants to try, I may have time to come back to this sometime in February.

@benmccann benmccann added the vite label Jan 16, 2022
@benmccann
Copy link
Member

Yes, this is almost certainly a bug in Vite. No worries that you don't have time to investigate a fix for this, but would you be able to file a bug for this in that repo? Tips on doing that here: https://github.com/sveltejs/kit#bug-reporting

There appears to be a related issue in the Vite issue tracker: vitejs/vite#5834

@Rich-Harris Rich-Harris added the bug Something isn't working label May 16, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone May 16, 2022
@bluwy
Copy link
Member

bluwy commented Jul 5, 2022

Tested this on latest SvelteKit, and it seems to be fixed now.

Capitalize test:

Cannot read properties of undefined (reading 'toUpperCase')
TypeError: Cannot read properties of undefined (reading 'toUpperCase')
    at Module.capitalize (/src/routes/_capitalize.js:4:22)
    at cap.svelte:5:6
    at Object.$$render (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/svelte@3.48.0/node_modules/svelte/internal/index.js:1758:22)
    at Object.default (root.svelte:43:39)
    at eval (/.svelte-kit/runtime/components/layout.svelte:8:41)
    at Object.$$render (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/svelte@3.48.0/node_modules/svelte/internal/index.js:1758:22)
    at root.svelte:37:37
    at $$render (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/svelte@3.48.0/node_modules/svelte/internal/index.js:1758:22)
    at Object.render (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/svelte@3.48.0/node_modules/svelte/internal/index.js:1766:26)
    at render_response (file:///Users/bjorn/Work/repros/kit-error/.svelte-kit/runtime/server/index.js:1271:27)

On line 1:

Cannot read properties of undefined (reading 'toUpperCase')
TypeError: Cannot read properties of undefined (reading 'toUpperCase')
    at /src/routes/_err.js:1:11
    at instantiateModule (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:50286:15)

However on reload:

`line` must be greater than 0 (lines start at line 1)
Error: `line` must be greater than 0 (lines start at line 1)
    at originalPositionFor$1 (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:16780:19)
    at /Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:50137:25
    at String.replace (<anonymous>)
    at /Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:50127:21
    at Array.map (<anonymous>)
    at ssrRewriteStacktrace (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:50126:10)
    at Object.ssrRewriteStacktrace (/Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/vite@2.9.13/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js:60149:20)
    at fix_stack_trace (file:///Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.357_svelte@3.48.0/node_modules/@sveltejs/kit/dist/vite.js:2091:29)
    at Object.get (file:///Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.357_svelte@3.48.0/node_modules/@sveltejs/kit/dist/vite.js:2235:19)
    at Object.hooks.handleError (file:///Users/bjorn/Work/repros/kit-error/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.357_svelte@3.48.0/node_modules/@sveltejs/kit/dist/vite.js:2170:18)

It might be an accidental double stack trace fix at

return error.stack ? vite.ssrRewriteStacktrace(error.stack) : error.stack;

and

vite.ssrFixStacktrace(error);

@bluwy bluwy removed the vite label Jul 5, 2022
@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Jul 19, 2022
Rich-Harris added a commit that referenced this issue Jul 21, 2022
* tidy up

* reset stack traces to avoid double fix - closes #3371

* use options.dev instead of __SVELTEKIT_DEV__ so that testing against src still works

* weird
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants