Exclude AbortError from unhandled rejections?

[benjamingr]

Hey, following a few discussions here tc39/proposal-cancellation#22 I'm wondering if AbortError should be excluded from generating an unhandledRejection. Aborting things isn't really an error (it's best effort) and the fact it's even an error and not a third-state running finally clauses only is unforunate.

In addition there are several places we (and DOM APIs) already silently remove handlers without rejecting anything with an AbortError when an AbortSignal is aborted.

@ronag @jasnell @mcollina @jakearchibald @ljharb @Linkgoron

[benjamingr]

(I have not made up my mind yet regarding this being a good/bad idea)

[jakearchibald]

Aborting things isn't really an error (it's best effort) and the fact it's even an error and not a third-state running finally clauses only is unforunate.

There was an attempt made in this direction, but it didn't go well in tc39 https://github.com/tc39/proposal-cancelable-promises/tree/0e769fda8e16bff0feffe964fddc43dcd86668ba.

I haven't used unhandledRejection much, so I don't have a strong feeling here. Does finally count as handled? silly question sorry

[benjamingr]

There was an attempt made in this direction, but it didn't go well in tc39

Yes, that was the reference. I was very much in favour of that effort back then :)

I haven't used unhandledRejection much, so I don't have a strong feeling here.

Unlike browsers - Node.js servers crash on uncaught exceptions (and unhandled rejections) to prevent cascading failures - in the browser this being uncaught isn't a big deal because in browsers AbortErrors will just cause console.logs when unhandled.

Intuitively - AbortError shouldn't do that and doesn't have the same problem that ignoring/suppressing errors does on the server but I am mostly waiting for someone to have a strong opinion on the topic.

[targos]

I don't have a clear opinion on this topic yet, but I feel like if we add a mechanism that allows to opt out of unhandled rejections for some classes, it should be public API and not only available through AbortError

[benjamingr]

I don't have a clear opinion on this topic yet, but I feel like if we add a mechanism that allows to opt out of unhandled rejections for some classes, it should be public API and not only available through AbortError

It could be a symbol for example or a brand/utility. bikeShed.markAsNotUnhandled(error) that takes an error and tells it to skip unhandledRejection/uncaughtException.

Though I think we're mostly missing someone with a strong opinion to say "this is a good/bad idea" now. I'll ping some friends who work on other platforms and ask.

[mcollina]

I don't think this is going to improve the developer experience of Node.js. What would be a code example that would benefit from unhandledRejection being skipped?

[benjamingr]

@mcollina good question, I think a lot of people use unhandledRejection for monitoring and it restarts the server. Referencing my experience from ±5 years ago basically:

• You have a server.
• You restart the server on unhandledRejection.
• Azure/AWS/GCP/DO/whatever restarts your server for an OS update. You get a SIGINT and gracefully shutdown.
• A lot of operations get cancelled because you propagate AbortSignals everywhere - some without explicit handling of errors because typically the operations they cancel failing is a programmer failure and should restart the server on its own.
• You wake up in the middle of the night because your code logged a thousand unhandledRejections very quickly and your monitoring saw your app's many fatal errors and assumed the server is failing to restart.

It's quite a specific case but one that happened to me in the past. If AbortErrors don't go through the same path you wouldn't have this problem. Is it worth it? Maybe?

I think a case where ignoring them is unsafe would settle this (in my book) towards not treating them differently. Better to wake up someone in the middle of the night and not fail catastrophically and the workaround of filtering yourself in a process.on('unhandledRejection' handler is acceptable.

[mcollina]

A lot of operations get cancelled because you propagate AbortSignals everywhere - some without explicit handling of errors because typically the operations they cancel failing is a programmer failure and should restart the server on its own.

Can you make an example for this? Either some code handles error or they don't. How could they handle every other cases but not those?

[ljharb]

Not handling a rejection isn’t an error either, so i don’t see why abort errors would be special.

[ronag]

Not handling a rejection isn’t an error either

Didn't we change this semi recently? I remember we had a vote and everything.

[ljharb]

@ronag a vote decides what node does, it doesn't change what the semantics of the language are intended to be.

There are no kinds of errors that the uncaught error listener treats specially, why should there be any kind of rejections that the unhandled rejection handler treats specially?

[ronag]

@ronag a vote decides what node does, it doesn't change what the semantics of the language are intended to be.

I'm confused. I was just referring to your statement Not handling a rejection isn’t an error either which as far as I know is not true in node. I'm not sure where language semantics come into the picture.

[ljharb]

@ronag ah, sorry for the confusion. what i mean is, Promise.reject();, per the language's semantics and its designers' intentions, is not an error; the programmer did not make a mistake. node certainly decided to treat that as an error with the unhandled rejections stuff.

The OP says:

Aborting things isn't really an error

and that seems to be discussing conceptual errors, so I was pointing out that no unhandled rejections are inherently conceptual errors.

[Jamesernator]

I have a related issue for Chrome Devtools, as in the browser one can just attach a "unhandledrejection" event handler and use it to filter out events (however this is probably unsolvable in such a way as such handlers aren't statically analyzable).

Node can't really do this though, I had an issue on Node to add a capability similar, but it doesn't sound like it would be compatible with Node's event model.

Though I think we're mostly missing someone with a strong opinion to say "this is a good/bad idea" now. I'll ping some friends who work on other platforms and ask.

Just from a quick look of some other languages, C# and Java both have some form of AbortException, these bubble up the stack per normal and need to be caught.

I'm not entirely sure for C# and Java, but Python is similar and throws a CancelledException which bubbles up the stack normally, however if you create a task object explictly rather than using await then calling .cancel() on the task won't bring the CancelledException into your own stack, i.e.:

import asyncio

async def forever():
    try:
        while True:
            await asyncio.sleep(1)
            print("Tick")
    except CancelledException:
        print("I saw cancellation")
        # We reraise the cancellation to propagate it up
        # Note that if you raise a different error here
        # it does get thrown in main()
        raise

async def main():
    task = asyncio.create_task(forever())
    await asyncio.sleep(5)
    # This doesn't throw an error, it only affects the stack within
    # the task that is run
    task.cancel()

asyncio.run(main())

This asynchronous stack stuff would be rather nice in JS, however it essentially Zones which have been rejected as TC39 before.

[benjamingr]

@ronag note that @ljharb's interpretation of what the language says is nuanced. The language does not specify what the host should do on unhandled rejections or uncaught exceptions - that is left to host-hooks to decide (e.g. HostPromiseRejectionTracker - 25.6.1.9 in the spec).

Different platforms (e.g. Node.js and the browser) do different things here because of different requirements (ux in browsers means errors just get logged, safety in Node.js means the process aborts).

The fact the language does not treat unhandled rejections in a certain way isn't evidence towards how Node.js should behave - it just means the language doesn't see it as its business to define these semantics - probably since it couldn't do so well given conflicting requirements from implementors.

[benjamingr]

@Jamesernator

This asynchronous stack stuff would be rather nice in JS, however it essentially Zones which have been rejected as TC39 before.

Note that both:

• Node.js (V8 really) already has asynchronous stack traces for async functions :)
• As for actual context or stacks - we have AsyncLocalStorage which is similar to zones (sans error handling) in its ability to carry around state in asynchronous context on-top of async_hooks. Whether or not that's a good idea? ¯_(ツ)_/¯

[ljharb]

@benjamingr you are completely correct that the only reason the spec doesn't explicitly ban what node is doing, is because nobody can figure out how to write the spec text that would ban it. The intent, however, is as I've stated (unhandled rejection hooks were added to the spec solely motivated by browsers being able to log unhandled rejections to the console), and node is violating the intent regardless of what loopholes the spec currently has.

Either way, uncaught errors are all treated the same in node, so I fail to see why it would make sense to treat any uncaught rejection differently from another.

[benjamingr]

@benjamingr you are completely correct that the only reason the spec doesn't explicitly ban what node is doing, is because nobody can figure out how to write the spec text that would ban it.

I am happy we agree that the spec does not prohibit or express an opinion on what runtimes do on unhandled rejections or uncaught exceptions. I personally think it would be unfortunate if it did since different runtimes have different requirements and needs.

The intent, however, is as I've stated (unhandled rejection hooks were added to the spec solely motivated by browsers being able to log unhandled rejections to the console), and node is violating the intent regardless of what loopholes the spec currently has.

I... don't believe that is the case? I mean, I can link to some 2014 esdiscuss posts discussing other cases - but honestly that is irrelevant I think? The spec is a formal document - Node.js agrees to conform to the spec - Node conforms to the spec either way. Anything that is "host environment" and not language related is mostly Node's purview.

Either way, uncaught errors are all treated the same in node, so I fail to see why it would make sense to treat any uncaught rejection differently from another.

That's a good point. I'm... not sure.

I think it's probably one of those cases where there are compelling arguments but the complexity of having an inconsistent API outweigh the benefits.

I'll go ahead and agree with the opinions expressed and close this in the meantime. Feel free to reopen if you want to bring it up again :)