Skip to content

Test runner swallows uncaughtException #44612

Closed
#45264
Closed
Test runner swallows uncaughtException#44612
#45264
Labels
good first issueIssues that are suitable for first-time contributors.test_runnerIssues and PRs related to the test runner subsystem.

Description

@tniessen
tniessen
opened on Sep 12, 2022

Version

v18.9.0

Platform

Linux ubuntuserver 5.15.0-1019-azure #24-Ubuntu SMP Tue Aug 23 15:05:55 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

test_runner

What steps will reproduce the bug?

import test from 'node:test';

test('foo', () => {
  setTimeout(() => { throw new Error(); }, 100);
});

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

The exit code of the process should indicate failure, not success. This is what happens without the experimental node:test and it is what other test runners do, too.

What do you see instead?

node --test swallows the error altogether. Not even a warning.

$ node --test test.mjs && echo -e "\nTest runner reported no error!"
TAP version 13
# Subtest: /home/tniessen/dev/github-44611/test.mjs
ok 1 - /home/tniessen/dev/github-44611/test.mjs
  ---
  duration_ms: 157.248203
  ...
1..1
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 159.519197

Test runner reported no error!

Without --test, at least there is a warning, but the process exit code still indicates success.

$ node test.mjs && echo -e "\nTest runner reported no error!"
(node:21021) ExperimentalWarning: The test runner is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
TAP version 13
# Subtest: foo
ok 1 - foo
  ---
  duration_ms: 0.910498
  ...
1..1
# Warning: Test "foo" generated asynchronous activity after the test ended. This activity created the error "Error" and would have caused the test to fail, but instead triggered an uncaughtException event.
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 103.150927

Test runner reported no error!

Additional information

No response

Activity

  1. tniessen
    added
    test_runnerIssues and PRs related to the test runner subsystem.
    on Sep 12, 2022

  2. MoLow commented on Sep 12, 2022

    @MoLow
    MoLow
    on Sep 12, 2022
    Member

    @tniessen isn't this a duplicate of #44611?

  3. tniessen commented on Sep 12, 2022

    @tniessen
    tniessen
    on Sep 12, 2022
    MemberAuthor

    @MoLow AFAICT, #44611 is only about the stack trace missing when the error is reported, but the test at least fails as expected. Here, the test runner reports success even though an uncaught exception occurred.

    (It could, of course, be the same bug causing two different problems.)

  4. timmolendijk commented on Sep 12, 2022

    @timmolendijk
    timmolendijk
    on Sep 12, 2022

    I think the rationale for this behavior is that the test finishes without an error, which makes the test runner conclude that it passes. After which any lingering async running code is just dismissed (and errors swallowed). This is to some extent alluded to in the docs.

    I am not sure whether this is the ideal behavior, but I can at least see how it could be considered by design.

  5. cjihrig commented on Sep 12, 2022

    @cjihrig
    cjihrig
    on Sep 12, 2022
    Contributor

    Without --test, at least there is a warning, but the process exit code still indicates success.

    When I originally wrote that code, I went back and forth on whether or not to change the exit code when this happens. I figured I'd wait and see if anyone complained. Someone complained now, so let's just change the exit code when a warning occurs.

  6. MoLow commented on Sep 12, 2022

    @MoLow
    MoLow
    on Sep 12, 2022
    Member

    @timmolendijk are you interested in creating a PR fixing this?
    if not I will

  7. timmolendijk commented on Sep 12, 2022

    @timmolendijk
    timmolendijk
    on Sep 12, 2022

    @MoLow Interested yes. Available not yet sure.

  8. MoLow commented on Sep 12, 2022

    @MoLow
    MoLow
    on Sep 12, 2022
    Member

    according to @cjihrig 's comment, the fix will require setting process.exitCode withing the uncaughtException handler.
    feel free to ping me if you need help

  9. MoLow
    added
    good first issueIssues that are suitable for first-time contributors.
    on Oct 27, 2022

  10. fossamagna commented on Oct 28, 2022

    @fossamagna
    fossamagna
    on Oct 28, 2022
    Contributor

    @MoLow I interest in creating a PR fixing this. If @timmolendijk not work on it yet, Can I take this up?

  11. MoLow commented on Oct 28, 2022

    @MoLow
    MoLow
    on Oct 28, 2022
    Member

    please do :)

  12. timmolendijk commented on Oct 28, 2022

    @timmolendijk
    timmolendijk
    on Oct 28, 2022

    @fossamagna Yeah please do, I hadn’t got around to it yet. Thanks!

  13. nodejs-github-bot
    closed this as completedin #45264on Nov 7, 2022

  14. 3 remaining items

  15. MoLow
    added a commit that references this issue on Dec 9, 2022

    test_runner: avoid swallowing of asynchronously thrown errors

    549b123
  16. fossamagna
    added a commit that references this issue on Jan 23, 2023

    test_runner: avoid swallowing of asynchronously thrown errors

    024450a
  17. MoLow
    added a commit that references this issue on Feb 25, 2023

    test_runner: avoid swallowing of asynchronously thrown errors

    9ec0a4e
  18. juanarbol
    added 2 commits that reference this issue on Mar 3, 2023

    test_runner: avoid swallowing of asynchronously thrown errors

    c662025

    test_runner: avoid swallowing of asynchronously thrown errors

    5fdf374
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    good first issueIssues that are suitable for first-time contributors.test_runnerIssues and PRs related to the test runner subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions