Repository navigation
async/await: nowait keyword? #13376
Description
I have a large project that has now been migrated over from javascript to typescript and we're very pleased with the results. Bugs have fallen dramatically with the improved compiler support in combination with rigorously written code.
Ours is a server-side product that runs under node (ts-node, actually). By its nature, it is highly asynchronous, so the move to Typescript had the major benefit of providing us access to async/await. It has been working really well, and dramatically reduced our code size and complexity.
The one problem it has introduced is that we occasionally miss putting an "await" keyword in front of calls to methods returning promises (i.e., other async functions) when inside an async function. This "send and forget" is something that one occasionally might want to use, but for the most part, this creates a lot of chaos in our design pattern, as in most cases, the call is to something that will complete asynchronously, but most of the time we need to wait for that to complete. (That method cannot be synchronous because it depends on external services that are asynchronous.) It is not uncommon in our case to have 5 or 10 await statements in a single method.
A missing "await" keyword can be hard to find -- especially when the method being called doesn't return anything that is needed by the caller. (If it returns something, type-checking will usually catch the mismatch between the type of the Promise returned from the call.)
Ideally, we could have a "nowait" keyword that would go where one would otherwise put an "await" to indicate to the Typescript compiler that the design intent is to explicitly NOT WAIT for the called method to be completing. The tsc compiler could have a new flag that controls whether to create a warning if a method call returns a Promise that is missing either "await" or "nowait". The "nowait" keyword would have no effect whatsoever on the compiled code.
Activity
aseemk commentedon Jan 16, 2017
I strongly agree with the problem this issue is trying to address. My colleagues and I continue to hit this all the time. (I'd estimate at least once a week.) And it frequently masks bugs.
It goes without saying that this only happens on
voidmethods. But a lot of times, thosevoidmethods are significant, e.g.validateFoo,checkAccess, asynctests, etc.I imagine the specific solution proposed in this issue (of a
nowaitkeyword) will be challenging sinceasync/awaitis an ES7 feature, not a TypeScript one. But here are other related issues:#10381
palantir/tslint#892
I'd be a fan of having this check built into the compiler rather than the linter, since it's truly a correctness issue, not a style issue.
Thanks to everyone for the consideration!
alitaheri commentedon Jan 17, 2017
I have recently started migrating a huge project from promises to async/await. having this feature can really ease such migrations. It's very hard to detect dangling promises until something goes terribly wrong with hours of trying to reproduce the issue only to find a function call was missing
await😱aluanhaddad commentedon Jan 17, 2017
This would be highly valuable but the problem with a
noawaitkeyword is that it introduces new expression level syntax.However, we can avoid this problem by taking the idea and introducing a flag,
--requireAwait(:bike:🏠), which would trigger an error if a top level expression of typePromise<T>within anasyncmethod is not awaited and its value is not part of an expression or an assignment.To suppress the error, the language could require a type assertion, which would otherwise be meaningless
There are some issues with this idea though.
const p = f()is currently valid but likely indicates an error so encouraging it could be badPromise<void>to return a value is generally preferable(but sometimes not possible or desirable).alitaheri commentedon Jan 17, 2017
Aluan Haddad (@aluanhaddad) I like that idea.
Since typescript is going to introduce decorators like
@@deprecatedhow about@@requireAwait? Might even be able to provide an implementation that actually catches these in run time. No more flags too 😁It will be opt-in.
Edit: or maybe opt-out with a flag:
--requireAwaitand@@ignoreAwaitaluanhaddad commentedon Jan 17, 2017
Ali Taheri Moghaddar (@alitaheri) that would be much cleaner. Did you mean
@@deprecated?alitaheri commentedon Jan 17, 2017
yes, I had forgotten that it was a bit different. Thanks for the reminder 😅 😅
aseemk commentedon Jan 17, 2017
Aluan Haddad (@aluanhaddad): I love your proposal. My colleague ryanwe has had a similar thought: this issue can be caught when a caller doesn't make use of a returned Promise.
I don't feel like the issues you list are a big deal. To me, it's a major step forward for the compiler to help guard against the 90% bug case, even if it doesn't address the last 10%. (The linter can help w/ that last 10%, e.g.
const p = f()and you don't make use of thatp.)Ali Taheri Moghaddar (@alitaheri): I might not understand your proposal. Where would I put this
@@requireAwaitdecorator? On every singleasyncmethod I have? (I see your edit now for flipping that to@@ignoreAwait👍 )RyanCavanaugh commentedon Jan 17, 2017
There's no need for
nowait- thevoidoperator already exists for a consuming an expression while producing no side effects (e.g. you could just writevoid doSomethingAsync();)kduffie commentedon Jan 17, 2017
Ryan Cavanaugh (@RyanCavanaugh) I think you are missing a key point. The problem is not about whether the promise gets executed. The problem is that the writer forgets to put the "await" in front of a call to an async method/function and gets a behavior that he/she is not expecting. So I believe this is a compiler warning/error issue only -- not a functional one. I'd be fine if this was "opt-in" in the sense that one would have to choose to turn on a compiler option to get warnings about calls to methods returning promises inside async functions that are unused. If using this compiler option, the 'nowait' keyword could be used if the writer has the (rare) need to invoke the method but not wait for the result.
aluanhaddad commentedon Jan 17, 2017
Kingston Duffie (@kduffie) I think what Ryan Cavanaugh (@RyanCavanaugh) is saying is that a flag is sufficient and that no new syntax is required. That is a great thing. I don't think he is dismissing the issue as it is now marked as in discussion.
aluanhaddad commentedon Jan 17, 2017
The funny thing my initial type level syntax suggestion, was going to involve asserting that the type of the expression was
voidwhich is a type error.
It never occurred to me to use
Which is far more elegant and is a JavaScript construct 😅
kduffie commentedon Jan 17, 2017
Ah. I see. Sorry for the confusion. He's saying that "void" would tell the compiler that you understood that promise returned is intentionally unused. With the new compiler flag present, one would get an error if there is neither an await nor a "void". Got it. Just fine with me.
aluanhaddad commentedon Jan 17, 2017
Kingston Duffie (@kduffie) I believe so. I think that would be a practical solution, but we will see.
83 remaining items
TimvdLippe commentedon Mar 7, 2023
FWIW I am familiar with the Puppeteer codebase and it doesn't surprise me that it would find a lot of issues. Floating promises are a major painpoint for DevTools and Puppeteer which adopted Promises in the same era. Therefore, I wouldn't immediately write them all off as false positives. That said, it is quite a list and it probably needs extensive analysis which are legitimately floating and which aren't.
jpike88 commentedon Mar 8, 2023
Ryan Cavanaugh (@RyanCavanaugh) I personally think by skimming through the examples you provided, they are coding in much lower level styles in which case they shouldn't be opting in to something like this. I would also call looping over a lot of floating promises, or firing off a lot of floating promises in succession, a code smell. Promise.all exists for a reason!
I wouldn't consider anyway your findings to be a high false positive rate, quite a few of these files are for typing purposes only, and aren't worth trying to pore over due to their inapplicability to any practical case that wouldn't drive someone mad trying to figure out.
I think the best way to treat this is that it's OK and perfectly legitimate (rarely!) to be firing promises off without directly handling their results in the same 'thread', but TypeScript should give you a way to distinguish between a promise that you are floating either by accident or on purpose, which currently is impossible to tell as no visual markers are enforced to qualify the intention of the Promise usage.
So in other words, those library maintainers if they choose to opt-in can just place the
voidoperator in front of any promise to pass the rule, and at the same time other developers will enjoy the benefit of understanding that the floating promises were written intentionally, with all the async complications that may follow. It would be easy to do and bring richer context to code in general, everybody wins.emilioplatzer commentedon Mar 9, 2023
I expect that in the current state of my codebase all of the floating promises that you can find are legitims (thanks for our testers). But I'm sure that all of the floating promises that you can find in the diffs of all commits in my repository are buggy ones.
I was thinking about how to indicate that a promise is not floating. I can use
voidhoping that will be the solution. But what if is not? Maybe I can add a new functionletItFloatto my code:And use it like:
Tomorrow I can replace
letItFloatwithvoid.tsconfig
And maybe this feature can be turned on in tsconfig and being false by default.
jpike88 commentedon Mar 27, 2023
We just ran into a problem again where a un-awaited promise was pushed to production (but managed to slip through automated and QA tests due to timing differences), which would have resulted in a very nasty, hard to diagnose bug. I only caught it because I decided to pore over the closed PRs a second time out of boredom. If discussion around this could intensify until a resolution is reached that would be ideal.
dsherret commentedon Jun 27, 2023
Detection of floating promises is probably my most wanted feature. I very often forget to await a promise and tracing down these kind of bugs without tooling can be difficult to spot. It's true there is a high false positive rate for this one, but I think it's good practice to explicitly opt-out of false positives to make it clear that was intended to the reader. For example, the C# compiler warns when not awaiting or assigning a task and I've always found that super helpful.
Ability for API authors to supress the error
Perhaps for APIs that are meant to be "fire and forget" and optionally awaited there could be a way for API authors to define that in order to supress this error for API consumers.
For example:
...where
Floatable<T extends PromiseLike>is a built-in type (maybe for most cases people could just usetype FloatablePromise<T> = Floatable<Promise<T>>).Calling Async in Sync
Regarding calling an async function in a sync function: yes, I believe this should error similarly to how it should in a top level module statement. It's super rare to call an async function from a sync one. I very commonly do this by accident. I'd rather be verbose opting out of false positives with a
voidkeyword in order to make my intent clear and catch bugs in scenarios where I didn't mean to do that.phaux commentedon Dec 4, 2024
or add error type to Promise and make
Promise<T, never>floatable by default.fregante commentedon Feb 18, 2025
Two things:
voidkeyword, which already means "the return value is not used". In this case the return value is a promise, so it makes perfect sense as a replacement.emilioplatzer commentedon Feb 18, 2025
fregante I think that
no-floating-promisesis not doing all the job. It is not complaining if you store the promise in a variable without awaitng it:For example if
sleepAndRunreturns a promise, thenconst y = sleepAndReturn(300,3);is not awaiting itand
no-floating-promisesis not complaining.full example:
fregante commentedon Feb 18, 2025
In that case it's not floating, you're assigning a promise to a variable, which is perfectly fine and unlikely to ever be part of TSC.
If you're looking for that part, it would however been a decent lint rule, e.g.
no-stored-promises. I'd open an issue on the TypeScript ESLint repo, if the request doesn't already exist.In reality you could also avoid the issue by using
const y: number = sleepAndReturn(), in which case TSC already throws an error (I know this isn't ideal nor automatic though)nth-commit commentedon Feb 21, 2025
It seems quite arbitrary to be concerned with ignored returned promises, but not with other ignored returned values. Consider this example:
Both the floating promise and this example fall into the error class:
I'm for the
voidsyntax, but can we apply it to everything returned value? It's simpler, consistent, and solves multiple issues.There's so many programming errors that this would help avoid
Here's another one:
mathijs81 commentedon Feb 21, 2025
I think ignoring a Promise is not as arbitrary as ignoring regular return values as in many cases, ignoring a promise will unexpectedly skip big chunks of code being executed at all.
Take the very common mistake
It's very counterintuitive that ignoring the 'return' value of storeInDB means that the code is not even executed, so nothing is actually stored in the db.
TimvdLippe commentedon Feb 21, 2025
There is precedence in other languages to implement such a feature in a checker: https://errorprone.info/bugpattern/CheckReturnValue Note that this checker implements it as opt-in. It doesn't force it on all methods, but only those marked. I have successfully used this feature for methods that were known harmful to ignore the value. To me it makes sense to replicate that in TypeScript as well.
nth-commit commentedon Feb 21, 2025
Mathijs Vogelzang (@mathijs81) If the function that's called returns a function, that the caller is expected to then call, then big chunks of code will not be executed. Non-async functions can still have just as critical side-effects.
joshuakb2 commentedon Feb 21, 2025
It's not clear what the mistake is here. I guess the programmer is supposed to expect each DB operation to happen in sequence, but as written they will happen in parallel instead.
That's actually the issue with ignoring promises. Not that code won't run, but that code won't run at the intended time.
emilioplatzer commentedon Feb 22, 2025
Michael Fry (@nth-commit) fregante I think I get your point.
Ignoring any return value can be a source of errors.
I prefer to be forced to add "something" in this case. Because there are a little few times when I really meant I want to drop. Adding that "something" doesn't hurts and helps a lot. ("something" could be a keyword or a hint or a @-comment).
I know some libraries are designed to return something "just in case you need it", well maybe we need to handle that adding some "ignore ignored return in that module".
Floating promises can be a source of errors.
Is not the same as ignoring return value. If you store a promise in a variable and you don't use it you also have a case of floating promise. That's why we need something different here. You can argue that having unused values is algo a source of errors and controlling that you are safe. Again, comparing that value with a boolean is not using the promise, is using the variable that has the promise.
A real case of use
You have this code:
with
checkPermission(user:string, name:string, mode:'high'):booleanbeing non async function.Then in a future refactor you make
checkPermission(user:string, name:string, mode:'high'):Promise<boolean>an async function.I know, I shouldn't do that. But that things happens
Now
showDatastill compiles but hasHighPermission is not longer aboolean, now is aPromise<boolean>.You can still say. But now TS is checking "always true conditions". Yes, yes. Maybe if you turn on absolutely all possible hints and options I'll catch all the floating promises. But in some legacy or cooperative project you cannot turn on all, because the effort to silence them will be huge.
I am still convinced that this is enough important to have its own check