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: correctly report error numbers in stack traces #619

Open
stevenvachon opened this issue Jun 30, 2017 · 28 comments
Open

fix: correctly report error numbers in stack traces #619

stevenvachon opened this issue Jun 30, 2017 · 28 comments

Comments

@stevenvachon
Copy link

stevenvachon commented Jun 30, 2017

When I take my mocha code out of nyc, the line numbers are correct, so I know that nyc is to blame.

stevenvachon/incomplete-url@9c6eb47

produces: https://travis-ci.org/stevenvachon/incomplete-url/jobs/248590761

I get the same result when adding source-map-support:

nyc --source-map --produce-source-map mocha test --require=source-map-support/register
@bcoe
Copy link
Member

bcoe commented Jul 29, 2017

@stevenvachon if I follow, line numbers are still incorrect with produce-source-map is enabled?

@kpdecker perhaps you could help debug a bit? I haven't played too much with this feature -- hopefully there hasn't been a regression.

@stevenvachon would definitely love to get this working for you; would also any help you can provide debugging, if you have any spare cycles.

@stevenvachon
Copy link
Author

@bcoe you followed correctly. I can maybe try to help debug.

@bcoe
Copy link
Member

bcoe commented Aug 1, 2017

@stevenvachon would definitely appreciate help digging into this; sounds like potentially a regression with produce-source-map.

@sneakertack
Copy link

I faced a similar issue with --produce-source-map not appearing to work a few days back, even with source-map-support (in my case, required in app code via require('source-map-support').install({hookRequire: true})).

I think I've gotten it working after adding both the --eager and --cache false flags. @stevenvachon maybe you could verify if that fixes things?

To me it feels like it has something to do with instrumenter caching - if I recall correctly, there was also some weirdness along the lines of:

  1. Remove those 2 flags.
  2. Error trace shows correct line number.
  3. Shift error-causing code 1 line up.
  4. Error trace shows wrong line number.
  5. Shift error-causing code back down.
  6. Error trace shows correct line number.

I may not have remembered the above steps/results entirely accurately, but it definitely felt anomalous so that could be a potential lead.

@bcoe
Copy link
Member

bcoe commented Aug 14, 2017

@sneakertack a similar class of bug has been reported by @isaacs' related to stack-traces getting out of alignment when using arrow-functions ... it might be worth diving a bit deeper into the source-maps being generated by Istanbul's instrumenter ... feels like something is out of whack.

@sompylasar
Copy link

I'm having similar line number issues with nyc, mocha, and ts-node – without babel, without explicit source-map-support/register (assuming ts-node takes care), regardless of tsconfig.json "sourceMap": true or "inlineSourceMap": true, regardless of nyc --produce-source-map.

    "nyc": "11.2.1",
    "ts-node": "3.3.0",
// A "test" command in `@my-company/ts-build-tools` 
// which ts-based packages use to build themselves.

    // http://web.archive.org/web/20170101073300/http://rundef.com:80/typescript-code-coverage-istanbul-nyc
    // http://rundef.com/typescript-code-coverage-istanbul-nyc
    const cliCommand = '${TOOLS_NODE_MODULES_DIR}/.bin/nyc';
    const cliArgs = ([
      '--require', '${TOOLS_DIR}/src/mocha-ts-node-register.js',
      '--extension=.ts',
      '--extension=.tsx',
      '--extension=.js',
      '--extension=.jsx',
      '--include=src/**',
      '--include=test/**',
      '--exclude=**/*.d.ts',
      '--all',
    ]).concat(
      (!shouldFailOnCoverageCheck ? [] : [
        '--check-coverage',
        '--lines', String(coverageCheckLinesThreshold),
      ])
    ).concat([
      '--reporter=lcov',
      '--reporter=text-summary',
      '--reporter=text',
      '${TOOLS_NODE_MODULES_DIR}/.bin/mocha',
      '-t', String(testsTimeout),
      '${PROJECT_DIR}/test/test.ts',
    ]);

    spawn(cliCommand, cliArgs, {}, onTestDone);
// @my-company/ts-build-tools/src/mocha-ts-node-register.js

const expandPaths = require('./paths').expandPaths;

require('ts-node').register({
  project: expandPaths('${PROJECT_DIR}/test/tsconfig.json'),
});

// some-ts-package/tsconfig.json

{
  "compilerOptions": {
    "target": "ES5",
    "lib": [ "ES5", "ES6", "ES2015" ],
    "module": "commonjs",
    "outDir": "./lib",
    "sourceMap": true,
    "listFiles": false,
    "listEmittedFiles": false,
    "pretty": true,
    "traceResolution": false,
    "declaration": true,
    "alwaysStrict": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "strictNullChecks": true,
    "downlevelIteration": true,
    "typeRoots": [
      "./node_modules/@my-company/ts-build-tools/node_modules/@types",
      "./node_modules/@types"
    ]
  },
  "include": [
    "./src/**/*.ts",
    "./typings/**/*.d.ts",
    "./node_modules/@my-company/ts-build-tools/shared/typings/**/*.d.ts"
  ],
  "exclude": [
    "./node_modules"
  ]
}

some-ts-package/test/tsconfig.json

{
  "extends": "../tsconfig.json",
  "compilerOptions": {
    "outDir": "../lib/test"
  },
  "include": [
    "../test/**/*.ts",
    "../typings/**/*.d.ts",
    "../node_modules/@my-company/ts-build-tools/shared/typings/**/*.d.ts"
  ],
  "exclude": [
    "../node_modules"
  ]
}

Example assert error in a test (obviously, the error is not at line 3 and character 11736):

      at Object.<anonymous> (test/censoredFileName.ts:3:11736)
      at step (test/censoredFileName.ts:1:32205)
      at Object.next (test/censoredFileName.ts:1:29481)
      at fulfilled (test/censoredFileName.ts:1:28026)
      at process._tickCallback (internal/process/next_tick.js:103:7)

Also these look related:

@bcoe bcoe changed the title Incorrect line numbers in error stacks fix: correctly report error numbers in stack traces Oct 4, 2017
@bcoe
Copy link
Member

bcoe commented Oct 4, 2017

It would be nice to, once and for all, correctly report line numbers in stack traces in as many common nyc configurations as possible.

A good start would be to add some unit tests to istanbul-lib-instrument. to test the simple cases.

@OmgImAlexis
Copy link

I'm having a similar issue with nyc and ava.

Ref: avajs/ava#1604

@bajtos
Copy link

bajtos commented Jan 23, 2018

We are experiencing similar problem in https://github.com/strongloop/loopback-next:

  • Both src and test is transpiled using TypeScript, external source maps are used

  • mocha.opts registers source-map-support

  • When the tests are run via nyc mocha, some of the error stack-trace frames are pointing to original TypeScript sources but others are pointing to transpiled JavaScript - see fix: apply source-maps to test errors loopbackio/loopback-next#884 (comment)

    at TestController.hello
      ([cut]/packages/rest/dist/test/integration/http-handler.integration.js:6:8854)
    at ControllerRoute.invokeHandler
      ([cut]/packages/rest/src/router/routing-table.ts:47:50)
    
  • When the tests are run via mocha, all error stack-trace frames are pointing to original TypeScript sources

    at TestController.hello
      ([cut]/packages/rest/test/integration/http-handler.integration.ts:471:17)
    at ControllerRoute.invokeHandler 
      ([cut]/packages/rest/src/router/routing-table.ts:318:46)
    

(cc @raymondfeng)

@shellscape
Copy link

shellscape commented Jan 23, 2018

Yeah this is 🍌 . Between running nyc mocha and mocha I'm running into a 71 line difference between the two. FWIW the solution proposed by @sneakertack didn't resolve the issue for me.

@bcoe given that I'm not running anything fancy, just straight up nyc mocha test/test.js --full-trace --exit this probably points to a regression.

@keenwon
Copy link

keenwon commented May 18, 2018

I have the same problem. Is there any solution?

@ianldgs
Copy link

ianldgs commented Jul 17, 2018

Having that problem too. Using nyc + mocha + ts-node. Source maps activated everywhere.


Edit:
found a workarround!

"scripts": {
  ...
  "test": "cross-env TS_NODE_FILES=true mocha --opts mocha.opts",
  "posttest": "cross-env TS_NODE_FILES=true nyc mocha --opts mocha.opts",
  ...
},

if tests fails, posttest won't run and stacktrace without nyc will be output.

@jbcpollak
Copy link

I believe we are having the same problem as @bajtos (Hi, we use Loopback 👋 !) - we are not using ts-node. We have TSC compile sourceMaps (as external .map files, not inline) and we have mocha --register source-map-support/register.

When we run mocha itself, everything works great. When we run nyc mocha, the stack traces get weird. Sometimes they show the TS file with a bad line number, sometimes they show the .js file.

@dipasqualew
Copy link

dipasqualew commented Nov 26, 2018

Is there any update on the matter? using @ianldgs workaround feels wrong as the test run twice and as he states it is just a workaround.

I believe this is a breaking issue on both the JavaScript and the TypeScript environment as being able to read meaningful stacktraces in unit and integration tests is a core reason for writing tests in the first place.

@bajtos
Copy link

bajtos commented Jan 25, 2019

@bcoe @kpdecker how can we help to get this issue fixed? Do you need a small(er) application to reproduce the problem? Something else? Please let us know!

@stale stale bot removed the stale label Jan 25, 2019
@istanbuljs istanbuljs deleted a comment from stale bot Jan 25, 2019
@stale stale bot added the stale label Mar 26, 2019
@JaKXz
Copy link
Member

JaKXz commented Mar 26, 2019

@bajtos sorry for the late reply: yes a minimal reproduction is a good first step! Also using the typescript config package with the latest versions of nyc etc will help.

Another troubleshooting step is to set cache: false in your nyc config and see if you can reproduce the issue.

alex028502 pushed a commit to alex028502/nyc-line-number-issue that referenced this issue Mar 29, 2019
@alex028502
Copy link

alex028502 commented Mar 29, 2019

@JaKXz I think this is the same issue that I was about to report. This example uses only mocha and nyc.

https://github.com/alex028502/nyc-line-number-issue

I have put --cache false. I hope that works.

@mbenna
Copy link

mbenna commented Mar 29, 2019

@alex028502, I confirm your small repro works exactly as described and fails as you describe. Nice work.

alex028502 pushed a commit to alex028502/nyc-line-number-issue that referenced this issue Mar 29, 2019
@alex028502
Copy link

@mbenna @JaKXz I have added a simpler example that gets a little closer to the bottom of the issue, and doesn't use mocha

@guikubivan
Copy link

Any update on this? We're using es6 code only and nyc mocha --recursive shows incorrect line numbers for stack-traces.

@coreyfarrell
Copy link
Member

This issue is blocked by evanw/node-source-map-support#239. The issue is that nyc source-maps are inline but node-source-map-support does not look at inline source-maps by default.

@csimi
Copy link

csimi commented Oct 11, 2019

I wonder why the produce-source-map option of nyc is not documented in the first place, source-map-support is useless without it.

@Raynos
Copy link

Raynos commented Jan 5, 2021

The solution I identified was to manually add the following lines to my test suite at the top of test/index.js

require('source-map-support').install({
  hookRequire: true
});

Then running nyc it will generate source maps out of the box, and my test code now correctly asks source-map-support to "load" the source maps nyc generates and now my stack traces for new Error() etc are correct.

npr nyc -a -r html -- node test/index.js

@bcoe
Copy link
Member

bcoe commented Jan 8, 2021

In newer versions of Node.js, you could also try running with --enable-source-maps.

@Raynos
Copy link

Raynos commented Jan 8, 2021

raynos at raynos-Precision-5530  
~/optoolco/_op-sdk on master*
$ npr nyc -a -r html -- node test/index.js 
TAP version 13
# setup
# sync upload

/home/raynos/optoolco/_op-sdk/src/helpers/concurrent-queue/index.js:13
    throw new Error('whoop')
          ^
Error: whoop
    at new ConcurrentQueue (/home/raynos/optoolco/_op-sdk/src/helpers/concurrent-queue/index.js:13:11)
    at Object.module.exports [as s3] (/home/raynos/optoolco/_op-sdk/src/buckets/sync/s3.js:58:17)
    at Test.<anonymous> (/home/raynos/optoolco/_op-sdk/test/buckets/sync/index.js:59:47)
raynos at raynos-Precision-5530  
~/optoolco/_op-sdk on master*
$ npr nyc -a -r html -- node --enable-source-maps test/index.js 
TAP version 13
# setup
# sync upload
/home/raynos/optoolco/_op-sdk/test/index.js:7
  process.nextTick(() => { throw err })
                           ^

Error: whoop
    at new ConcurrentQueue (/home/raynos/optoolco/_op-sdk/src/helpers/concurrent-queue/index.js:4:413)
        -> /home/raynos/optoolco/_op-sdk/src/helpers/concurrent-queue/index.js:13:11
    at Object.module.exports [as s3] (/home/raynos/optoolco/_op-sdk/src/buckets/sync/s3.js:14:124)
        -> /home/raynos/optoolco/_op-sdk/src/buckets/sync/s3.js:58:17
    at async Test.<anonymous> (/home/raynos/optoolco/_op-sdk/test/buckets/sync/index.js:59:47)

Can confirm that --enable-source-maps at least with node 14 appears to work. it just took me a moment to parse the differently formatted stack traces.

@jrichardsz
Copy link

In newer versions of Node.js, you could also try running with --enable-source-maps.

How can we use this command?

I tried this without success

nyc --enable-source-maps --reporter=html --reporter=json-summary mocha ...

@jrichardsz
Copy link

This worked for me

https://stackoverflow.com/a/77774805/3957754

@khaled4vokalz
Copy link

So after 7 years.... The problem remains....this is sadly unacceptable for a tool like this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests