Skip to content

async/await: nowait keyword? #13376

Open
Open

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

  1. aseemk commented on Jan 16, 2017

    @aseemk

    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 void methods. But a lot of times, those void methods are significant, e.g. validateFoo, checkAccess, async tests, etc.

    I imagine the specific solution proposed in this issue (of a nowait keyword) will be challenging since async/await is 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!

  2. alitaheri commented on Jan 17, 2017

    @alitaheri

    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 😱

  3. aluanhaddad commented on Jan 17, 2017

    @aluanhaddad
    Contributor

    This would be highly valuable but the problem with a noawait keyword 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 type Promise<T> within an async method 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

    declare function f(): Promise<void>;
    
    // --requireAwait
    async function g() {
      f(); // Error
      f() as {}; // OK
      await f(); // OK
      const p = f(); // OK
      p; // Error
      p as {}; // OK
      Promise.all([f(), p]); // Error because of .all
      await Promise.all([f(), p]); // OK
      return f(); // OK
    }

    There are some issues with this idea though.

    1. It adds a new flag
    2. const p = f() is currently valid but likely indicates an error so encouraging it could be bad
    3. Type assertions look weird in unused expressions.
    4. Stylistically, refactoring the Promise<void> to return a value is generally preferable(but sometimes not possible or desirable).
    5. Probably a number of things I have not considered.
  4. alitaheri commented on Jan 17, 2017

    @alitaheri

    Aluan Haddad (@aluanhaddad) I like that idea.

    Since typescript is going to introduce decorators like @@deprecated how 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: --requireAwait and @@ignoreAwait

  5. aluanhaddad commented on Jan 17, 2017

    @aluanhaddad
    Contributor

    Ali Taheri Moghaddar (@alitaheri) that would be much cleaner. Did you mean @@deprecated?

  6. alitaheri commented on Jan 17, 2017

    @alitaheri

    yes, I had forgotten that it was a bit different. Thanks for the reminder 😅 😅

  7. aseemk commented on Jan 17, 2017

    @aseemk

    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 that p.)

    Ali Taheri Moghaddar (@alitaheri): I might not understand your proposal. Where would I put this @@requireAwait decorator? On every single async method I have? (I see your edit now for flipping that to @@ignoreAwait 👍 )

  8. RyanCavanaugh commented on Jan 17, 2017

    @RyanCavanaugh
    Member

    There's no need for nowait - the void operator already exists for a consuming an expression while producing no side effects (e.g. you could just write void doSomethingAsync();)

  9. kduffie commented on Jan 17, 2017

    @kduffie
    Author

    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.

  10. aluanhaddad commented on Jan 17, 2017

    @aluanhaddad
    Contributor

    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.

  11. aluanhaddad commented on Jan 17, 2017

    @aluanhaddad
    Contributor

    The funny thing my initial type level syntax suggestion, was going to involve asserting that the type of the expression was void

    async function f() {
      returnsPromise() as void;
    }

    which is a type error.
    It never occurred to me to use

    async function f() {
      void returnsPromise();
    }

    Which is far more elegant and is a JavaScript construct 😅

  12. kduffie commented on Jan 17, 2017

    @kduffie
    Author

    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.

  13. aluanhaddad commented on Jan 17, 2017

    @aluanhaddad
    Contributor

    Kingston Duffie (@kduffie) I believe so. I think that would be a practical solution, but we will see.

  14. 83 remaining items

  15. TimvdLippe commented on Mar 7, 2023

    @TimvdLippe
    Contributor

    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.

  16. jpike88 commented on Mar 8, 2023

    @jpike88

    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 void operator 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.

  17. emilioplatzer commented on Mar 9, 2023

    @emilioplatzer

    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 void hoping that will be the solution. But what if is not? Maybe I can add a new function letItFloat to my code:

    // @ts-expect-error: letting float a promise on purpose
    function letItFloat(Promise<any> _) {}

    And use it like:

    async function runForever() {
       while (var x = await producer()) {
          await consumer(x)
       }
    }
    
    letItFloat(runForever());

    Tomorrow I can replace letItFloat with void .

    tsconfig

    And maybe this feature can be turned on in tsconfig and being false by default.

  18. jpike88 commented on Mar 27, 2023

    @jpike88

    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.

  19. dsherret commented on Jun 27, 2023

    @dsherret
    Contributor

    Probably more work needs to be done here to figure out a rule that doesn't have a high false positive rate, or make the case that at least one of the hits in that PR are good finds. I think it's a hard sell that webpack has dozens of code smells.

    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:

    async function serve1(handler: ...): Promise<void> {
      // ...
    }
    
    async function serve2(handler: ...): Floatable<Promise<void>> {
      // ...
    }
    
    // error, unawaited promise (add `void` to supress)
    serve1(() => { ... });
    
    // no error, promise is floatable
    serve2(() => { ... });

    ...where Floatable<T extends PromiseLike> is a built-in type (maybe for most cases people could just use type 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 void keyword in order to make my intent clear and catch bugs in scenarios where I didn't mean to do that.

  20. phaux commented on Dec 4, 2024

    @phaux

    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.

    ...where Floatable<T extends PromiseLike> is a built-in type (maybe for most cases people could just use type FloatablePromise<T> = Floatable<Promise<T>>).

    or add error type to Promise and make Promise<T, never> floatable by default.

  21. fregante commented on Feb 18, 2025

    @fregante

    Two things:

    • this is already possible via https://typescript-eslint.io/rules/no-floating-promises/
    • there's no need to introduce a new keyword, the plugin managed via the existing void keyword, 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.
  22. emilioplatzer commented on Feb 18, 2025

    @emilioplatzer

    fregante I think that no-floating-promises is not doing all the job. It is not complaining if you store the promise in a variable without awaitng it:

    For example if sleepAndRun returns a promise, then
    const y = sleepAndReturn(300,3); is not awaiting it
    and no-floating-promises is not complaining.

    full example:

    async function sleepAndReturn<T>(ms:number, returned:T):Promise<T>{
        return new Promise((resolve)=>{
            setInterval(function(){
                resolve(returned);
            },ms)
        });
    }
    async function main(){
        sleepAndReturn(100,'X');
        const x = await sleepAndReturn(100, 'z');
        const y = sleepAndReturn(300,3);
        console.log(x,y);
    }
    main();
  23. fregante commented on Feb 18, 2025

    @fregante

    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)

  24. nth-commit commented on Feb 21, 2025

    @nth-commit

    It seems quite arbitrary to be concerned with ignored returned promises, but not with other ignored returned values. Consider this example:

    class BankBalance {
      private balance = 0;
    
      public deposit(amount: number): void {
        calculateNewBalance(amount, '+'); // Programming error: it looks like it should update the balance but it actually does nothing
      }
    
      private calculateNewBalance(delta: number, direction: '+' | '-'): number {
        return direction === '+' ? this.balance + delta : this.balance - delta;
      }
    }
    

    Both the floating promise and this example fall into the error class:

    You, the caller, should have done something with the returned value, but didn't.

    I'm for the void syntax, 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:

    const foos = bars.map(bar => { mapToFoo(bar) });
    
  25. mathijs81 commented on Feb 21, 2025

    @mathijs81

    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

    async function storeInDb(data: string) {
       await db.store(Date.now(), data);
    }
    
    async function main() {
      const someData = [ 'a', 'b', 'c' ];
      someData.forEach((data) => storeInDb(data));
    }
    

    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.

  26. TimvdLippe commented on Feb 21, 2025

    @TimvdLippe
    Contributor

    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.

  27. nth-commit commented on Feb 21, 2025

    @nth-commit

    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.

    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.

  28. joshuakb2 commented on Feb 21, 2025

    @joshuakb2
      someData.forEach((data) => storeInDb(data));
    

    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.

  29. emilioplatzer commented on Feb 22, 2025

    @emilioplatzer

    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:

    async function showData(name:string, user:string){
        const data = await getData(name);
        const hasHighPermission = checkPermission(user, name, 'high');
        if (hasHighPermission) {
            display([data.basic, data.high]);
        } else {
            display([data.basic]);
        }   
    }

    with checkPermission(user:string, name:string, mode:'high'):boolean being 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 showData still compiles but hasHighPermission is not longer a boolean, now is a Promise<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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Awaiting More FeedbackThis means we'd like to hear from more people who would be helped by this featureSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions