deno_lint icon indicating copy to clipboard operation
deno_lint copied to clipboard

[deno lint] Uncaught Promise Warning

Open Skillz4Killz opened this issue 5 years ago • 3 comments

Would it be possible to somehow warn a user when a Promise is not properly handled? Deno will always crash on uncaught errors. I guarantee there will be many users that will have an issue because their project continues to crash because they forgot to catch a promise. Somewhere in the hundreds, thousands, or millions of lines of code for some project someone is gonna forget a promise and have it crash in production.

Any sort of warning system during compile time to say hey u have an unhandled promise, this could crash your project. Please catch it.

Skillz4Killz avatar May 07 '20 15:05 Skillz4Killz

Edit: nevermind, I see that this issue is about deno-lint. Detecting unhandled rejections for the general case in compile time is the halting problem :]

It's possible to warn in some cases in compile time (when it's easy to detect that a promise was created but not returned or awaited). I think that would be a cool feature :]


Old:

Doesn't deno let you add error event handlers on the global object like a browser? I'd expect that to be the behavior.

For what it's worth, ignoring unhandled rejections can be much more dangerous than crashing. One common pattern we've seen users experience in Node.js is:

// node uses EventEmitter and not EventTarget
emitter.addEventListener('completed', () => {
  logger.log('releasing handle');
  somethingThatThrows();
  dbHandle.release();
});

Here, Node.js will crash because of an uncaught exception. However, as soon as it's made an async function:

// node uses EventEmitter and not EventTarget
emitter.addEventListener('completed', async () => {
  logger.log('releasing handle');
  somethingThatThrows(); // with or without await beforehand
  dbHandle.release();
});

That would make node warn an unhandled rejection instead of error out. The main issue with that is that it's really easy to cause a cascading failure this way (the DB runs out of handles and other services can't connect) and it's incredibly challenging to write code in a way that's safe when not crashing on unhandled rejections.

If you want to suppress an unhandled rejection you can .catch(() => {}) it.

benjamingr avatar May 07 '20 17:05 benjamingr

I recently found many unhandled assertRejects() calls, even in deno_std itself. People very often forget to put await before them.

kt3k avatar May 23 '22 12:05 kt3k

I recently found many unhandled assertRejects() calls, even in deno_std itself. People very often forget to put await before them.

I think as a stop-gap solution, we could warn about it for TypeScript files if there's an explicit Promise<T> return type specified. However it is quite involved to do so, as you currently lint files in isolation and adding support for this would require to store information cross-files.

@dsherret any ideas here?

bartlomieju avatar May 23 '22 14:05 bartlomieju