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

setTimeout(function(){throw null},0) crashes toplevel #12373

Closed
DemiMarie opened this issue Apr 12, 2017 · 7 comments
Closed

setTimeout(function(){throw null},0) crashes toplevel #12373

DemiMarie opened this issue Apr 12, 2017 · 7 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. domain Issues and PRs related to the domain subsystem. repl Issues and PRs related to the REPL subsystem.

Comments

@DemiMarie
Copy link

  • Version: v6.10.0
  • Platform: Linux localhost.hsd1.tn.comcast.net 4.10.8-200.fc25.x86_64 #1 SMP Fri Mar 31 13:20:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: timer

If I run

setTimeout(function() { throw null }, 0)

the REPL crashes with

TypeError: Cannot read property 'stack' of null
    at Domain.<anonymous> (repl.js:392:17)
    at emitOne (events.js:96:13)
    at Domain.emit (events.js:188:7)
    at Domain._errorHandler (domain.js:97:23)
    at process._fatalException (bootstrap_node.js:293:33)

Obviously such code is buggy (and would normally cause a crash). However, I don’t think that it should crash the toplevel, hence this report.

@vsemozhetbyt vsemozhetbyt added the repl Issues and PRs related to the REPL subsystem. label Apr 12, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 12, 2017

Can reproduce on Windows with 8.0.0rc:

> repl.js:291
    } else if (e.stack && self.replMode === exports.REPL_MODE_STRICT) {
                ^

TypeError: Cannot read property 'stack' of null
    at Domain.debugDomainError (repl.js:291:17)
    at emitOne (events.js:115:13)
    at Domain.emit (events.js:210:7)
    at Domain._errorHandler (domain.js:118:23)
    at process._fatalException (bootstrap_node.js:311:33)

@refack refack added confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors. labels Apr 12, 2017
@refack
Copy link
Contributor

refack commented Apr 12, 2017

repro on Windows with v4 v6, v7, v8, Ubuntu v7

@Fishrock123 Fishrock123 added domain Issues and PRs related to the domain subsystem. and removed good first issue Issues that are suitable for first-time contributors. labels Apr 13, 2017
@Fishrock123
Copy link
Contributor

Looks like a domain thing? I'm removing good first contrib because I feel like this won't be particularly clear.

@refack
Copy link
Contributor

refack commented Apr 13, 2017

Looks like a domain thing? I'm removing good first contrib because I feel like this won't be particularly clear

IMHO just needs an e && as in

  repl.js:291
    } else if (e && e.stack && self.replMode === exports.REPL_MODE_STRICT) {

@XadillaX
Copy link
Contributor

I think this issue should be closed?

@bnoordhuis
Copy link
Member

I don't think this was actually fixed, #12400 wasn't merged.

@benjamingr
Copy link
Member

Seeing as this is unhandled - I'll take a shot.

@benjamingr benjamingr self-assigned this Jul 16, 2017
benjamingr pushed a commit to benjamingr/io.js that referenced this issue Jul 17, 2017
Previous behavior was to assume an error is a proper error in the
repl module. A check was added to not terminate the process on thrown
repl errors that are `null` or `undefined`.

PR-URL: nodejs#14306
Fixes: nodejs#12373
Reviewed-By: Anna Henningsen <anna@addaleax.net>
benjamingr pushed a commit that referenced this issue Jul 19, 2017
Previous behavior was to assume an error is a proper error in the
repl module. A check was added to not terminate the process on thrown
repl errors that are `null` or `undefined`.

PR-URL: #14306
Fixes: #12373
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com
addaleax pushed a commit that referenced this issue Jul 22, 2017
Previous behavior was to assume an error is a proper error in the
repl module. A check was added to not terminate the process on thrown
repl errors that are `null` or `undefined`.

PR-URL: #14306
Fixes: #12373
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com
Fishrock123 pushed a commit that referenced this issue Jul 24, 2017
Previous behavior was to assume an error is a proper error in the
repl module. A check was added to not terminate the process on thrown
repl errors that are `null` or `undefined`.

PR-URL: #14306
Fixes: #12373
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. domain Issues and PRs related to the domain subsystem. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants