search

istanbuljs / nyc

the Istanbul command line interface
https://istanbul.js.org/
ISC License
5.58k stars 358 forks source link

fix: correctly report error numbers in stack traces #619

Open stevenvachon opened 7 years ago

stevenvachon commented 7 years ago

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

https://github.com/stevenvachon/incomplete-url/commit/9c6eb47e3b98916b3697af3ad472f7cd73ffffd7

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 commented 7 years ago

@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 commented 7 years ago

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

bcoe commented 7 years ago

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

sneakertack commented 7 years ago

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. 0) Error trace shows correct line number. 0) Shift error-causing code 1 line up. 0) Error trace shows wrong line number. 0) Shift error-causing code back down. 0) 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 commented 7 years ago

@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 commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

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

Ref: https://github.com/avajs/ava/issues/1604

bajtos commented 6 years ago

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

(cc @raymondfeng)

shellscape commented 6 years ago

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 commented 6 years ago

I have the same problem. Is there any solution?

ianldgs commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

@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!

JaKXz commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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

alex028502 commented 5 years ago

@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 commented 5 years ago

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

coreyfarrell commented 5 years ago

This issue is blocked by https://github.com/evanw/node-source-map-support/issues/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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Raynos commented 3 years ago 
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 commented 8 months ago

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 commented 8 months ago

This worked for me

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

khaled4vokalz commented 3 months ago

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

TwitchBronBron commented 1 month ago

I've been having this issue periodically across my projects, and today I finally fixed it for one of the projects.

I run nyc, mocha, and ts-node together on NodeJS v16.20.2. My solution was to remove source-map-support/register from my nyc configuration. (I guess it's already registered from my ts-node or mocha configurations?)

image

Hope that helps someone. 😄