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

Fatality of uncaught exceptions in NodeJS #6

Closed
rkatic opened this issue Jul 13, 2013 · 8 comments
Closed

Fatality of uncaught exceptions in NodeJS #6

rkatic opened this issue Jul 13, 2013 · 8 comments

Comments

@rkatic
Copy link
Collaborator

rkatic commented Jul 13, 2013

The paragraph about uncaught exceptions in the README.md says:

If a task throws an exception, it will not interrupt the flushing of high-priority tasks. The exception will be postponed to a later, low-priority event to avoid slow-downs, when the underlying JavaScript engine will treat it as it does any unhandled exception.

It explains correctly the behavior in browsers, but it is not true in NodeJS. In NodeJS, uncaught exceptions are fatal (exits the process immediately), unless domains (or the "uncaoughtException" event) are used. ASAP, currently follows such "error policy".

Now, we have two options: (1) to fix the paragraph in the README.md, or (2) to make ASAP behave in NodeJS exactly as in browsers, "braking" the "error policy" of NodeJS.

@domenic
Copy link
Collaborator

domenic commented Jul 13, 2013

I might be misunderstanding, but the readme seems perfectly accurate to me. In particular,

when the underlying JavaScript engine will treat it as it does any unhandled exception

seems to exactly match what asap does.

@kriskowal
Copy link
Owner

I do not think that the README is inaccurate, but I do agree that we should clarify what “as it does any unhandled exception” means for NodeJS, specifically with regard to our respect for domains.

@rkatic
Copy link
Collaborator Author

rkatic commented Jul 13, 2013

@domenic that part is fine, it's the rest that is untrue:

If a task throws an exception, it will not interrupt the flushing of high-priority tasks. The exception will be postponed to a later, low-priority event to avoid slow-downs, when...

@domenic
Copy link
Collaborator

domenic commented Jul 13, 2013

Oh hmm, looking at the code, you're right:

https://github.com/kriskowal/asap/blob/master/asap.js#L30-L47

Weird.

@domenic
Copy link
Collaborator

domenic commented Jul 13, 2013

I don't have a strong opinion on which behavior is better, as I'm still holding out hope for removing all exception handling from asap.

@kriskowal
Copy link
Owner

Oh, I see. Yeah, that’s different.

@rkatic
Copy link
Collaborator Author

rkatic commented Jul 14, 2013

ASAP is already at version 1.0.0, and I think documentation should correctly reflect the current state of the code. Apparently there will not be a consensus very soon, so I think we should fix the paragraph as soon as possible, and to revert it in future if needed. I am going to create a new branch for that.

@rkatic
Copy link
Collaborator Author

rkatic commented Feb 26, 2014

It seems I forgot to close this one.

@rkatic rkatic closed this as completed Feb 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants