deno_lint icon indicating copy to clipboard operation
deno_lint copied to clipboard

Feature request: no-floating-promises

Open callionica opened this issue 3 years ago • 8 comments

I'd love to get a warning when I miss await from a call to a Promise-returning function.

callionica avatar Sep 07 '20 17:09 callionica

I really love and desperately need this rule! However, I think this error should only occur when your trying to use the promise for something.

// ERROR
if (returnAPromise()) {

}

// ERROR
const test = returnAPromise();
if (test) {

}

// VALID
const test = 5;
doSomethingWithPromise();
return test;

I would not want to see EVERY promise required to be awaited. A lot of times you don't need to wait for the end result of the promise you just need it done. You don't need for the function to finish updating the lastUpdatedAt in the db to return the value of 5.

Skillz4Killz avatar Nov 14 '20 17:11 Skillz4Killz

I would not want to see EVERY promise required to be awaited.

I disagree. In the majority of cases, async ops are network calls. These may always fail, which would kill the process. That is never the desired behaviour.

I would be open to accept code as valid that has a floating promise with .catch installed on it. Even then I think it would be better to be more explicit, and add a comment ignoring the linting error.

KnorpelSenf avatar Nov 13 '22 13:11 KnorpelSenf

Came here to request the same thing.

It's great that deno test will tell you about unresolved promises at test time. But

  1. That only catches issues if you have sufficient test coverage. (and mocked tests might not have the same timings)

  2. Even when you do get test failures due to unresolved async operations, it can still be difficult to track down the source.

You have to make sure await is applied throughout the entire call chain, which can be a large surface area to manually check. It would be nice if we had a built-in tool to help with this!

Ex: https://palantir.github.io/tslint/rules/no-floating-promises/

NfNitLoop avatar May 10 '23 20:05 NfNitLoop

I would love to have this rule, it would provide so much more safety to many projects

denizdogan avatar May 11 '23 08:05 denizdogan

+1! This almost just bit me horribly. I would love if there was some way to make this work.

rmnoon avatar Jun 16 '23 15:06 rmnoon

I would not want to see EVERY promise required to be awaited

Agreed

I would be open to accept code as valid that has a floating promise with .catch installed on it.

You would be forced into some kind of error handling right there then. Not convenient.

What you want is to be concise with your intent. And you don't want to await sometimes. But you do want to explicitly say that "I am deliberately not awaiting this"

Having said that, casting to void works well. No need to await, no need to install a fake catch handler.

aliak00 avatar Dec 15 '23 12:12 aliak00

casting to void works well

If this code errors, your process will die. So either you need to do error handling inside the logic so that it never throws, or you do indeed need a catch handler on the promise.

But either way, it doesn't look like deno lint will be able to support such a rule anytime soon, hence rendering this question irrelevant. I guess we first need type checking in Rust, and I don't know when SWC will make this a reality, if ever.

KnorpelSenf avatar Dec 15 '23 14:12 KnorpelSenf

If this code errors, your process will die.

Correct. Which is IMO better than an empty catch handler that does nothing without any forethought (which will be what will happen if you enforce "fake-handling" errors - as people will just do minimal work to satisfy the lint). If "keep on going even if errors" is important, then you can always set a handler for unhandledrejection.

But yeah, sucks that Deno won't support this anytime soon :( it bit me twice yesterday (didn't know I was calling an async function)

aliak00 avatar Dec 19 '23 09:12 aliak00