Multiple issues with having stack traces from console.error warn and log

yuatpocketgems commented 4 years ago

🐛 Bug Report

Several issues I am running into: 1. Feature #9741 is not compatible with our own test utilities for prettifying test logs and stack traces. The feature introduced in #9741 is not a direct replacement of our utils either so we will need to keep our own version.

An example of our unit test output.

Test: test case
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)
    | 200 OK
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)
    | Some warning
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)
    | at func1 (file.js:12:12)
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)
    | at func2 (file2.js:23:23)
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)
    | at func3 (file3.js:34:34)
        at Object.write (../../node_modules/pino/lib/tools.js:240:25)
        at Pino.write (../../node_modules/pino/lib/proto.js:161:15)
        at Pino.warn (../../node_modules/pino/lib/tools.js:39:26)

our logs are at the first 2 indentation levels, lines mentioning pino were added by jest.

There might be other legitimate use cases that makes this new feature is unwanted. Can we consider to have people opt-in rather than opt-out of the feature?

Rationale being:

For projects that are starting from scratch, it's not hard for them to find the flag. And if they missed the flag from documentation, it is trivial work to log stack traces for errors.
For existing projects, it's much less work to trace back the origin of these new logs when there are many 3rd party libraries to investigate.

2. Similar to the previous example output, we used console.log, but the full stack trace is printed.

The changes in #9741 introduces backward compatibility issues with other jest installations. We have our own custom reporter installed which uses a jest version prior to 25.4.0 (prior to #9741), while the test environment's main jest version is the latest.

Our custom reporter uses jest's getConsoleOutput, and prior to 25.4.0, the origin of a buffered log is dumped to stdout as shown here. Hence regardless of log level, the entire call stack was printed.

Can we consider supporting different versions of getConsoleOutput as they are functionally different, and deprecate older versions gradually with some warnings?

3. Global flag noStackTrace is ignored by getConsoleOutput. See snippet. Error and warn will always have the stack trace printed. This is a problem for us, since we use error and warn to print stack traces too. Can we have the local variable noStackTrace take the value from global config?

4. For log and info level, the top frame is till emitted despite having variable noStackTrace set to true. See snippet.

If noStackTrace is a global configuration, then it should come before any built in rules, so that the flag behaves truely as an override, unless I am missing some use cases here.

To Reproduce

Steps to reproduce the behavior: For 1, 3, 4. get jest version 25.4.0+. Calling console.log, console.warn shows the problem.

For 2, use old jest version (prior to 25.4.0 to build custom reporters that also uses getConsoleOutput for generating the final output.

Expected behavior

Potentially breaking feature should be opted-in not opted-out
Co-existing jest versions should be allowed. (at least with the same major version) 3 & 4. noStackTrace flag should actually help opt-out of the stack trace feature.

envinfo

npx: installed 1 in 1.623s

System: OS: macOS 10.15.4 CPU: (12) x64 Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz Binaries: Node: 13.12.0 - /usr/local/bin/node Yarn: 1.22.4 - /usr/local/bin/yarn npm: 6.14.4 - /usr/local/bin/npm

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

jestjs / jest