Feature request: no-floating-promises

[callionica]

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

[Skillz4Killz]

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.

[KnorpelSenf]

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.

[NfNitLoop]

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/

[denizdogan]

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

[rmnoon]

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

[aliak00]

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.

[KnorpelSenf]

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.

[aliak00]

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)

[KnorpelSenf]

What if this rule can be implemented with a best-effort approach? It should be fairly easy to detect all cases where

• a method is called that has an explicit return type annotation with Promise<_>
• an async method is called
• a method is called that has a branch in which it returns Promise.resolve, Promise.reject, or other helper methods known to return promises.

This does not require type checking. It can be implemented fully in Rust and should catch 99 % of cases in real world code.

@bartlomieju if this is acceptable, then the label requires type checking can be removed.

[NfNitLoop]

To expand on something @KnorpelSenf said previously:

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.

That "would kill the process" didn't quite register with me at the time. I'd forgotten that Deno's default behavior is to kill the entire process if an unhandledrejection is encountered. That means that if any of your dependencies forgets to await a single Promise, your process crashes. I ran into this issue just yesterday[^2].

Having this lint (or at least a best-effort version) available by default in Deno would greatly improve the quality of Deno/JSR packages and the Deno ecosystem as a whole.

[^2]: TBF, the dependency I was using was my own. 🤦 But it's still an example of how this problem can affect more than your own package.

[KnorpelSenf]

I'd forgotten that Deno's default behavior is to kill the entire process

In fact, recent versions of Node.js behave the same way.

Also, both runtimes let you install global handlers that can catch unhandled rejections and let you suppress the errors so that your process does not die. That being said, it is probably a bad idea to do that—if you have an unhandled rejection, you want to find out about it as soon as possible, and killing the process is a great way to make you very aware of it. So instead of suppressing the error, it's better to catch the rejection.

[phaux]

BTW you can just run eslint on your Deno project and it almost works. It will use default TypeScript so it won't understand HTTP or npm imports so it's very limited.

You can specify alternative TypeScript implementation in your eslint config, but Deno's implementation is written partially in Rust (AFAIK) so it's not simple either.

The only option is to reimplement Deno's TS fork in pure js, using for example TS VFS. A script that walks recursively all the modules and adds them to the VFS before creating the TS Program would be needed.