
Beware of Async/Await - brundolf
https://www.brandonsmith.ninja/blog/async-await
======
jakear
Curious - did anyone look at the first example and not immediately see that
the requests would occur in series? IMO the “await” actually makes this much
_more_ explicit.

~~~
faizshah
I did have this misunderstanding when relearning Promises and Async/Await in
JS. The misunderstanding is actually not about await but about when a Promise
is executed. The question you ask yourself when learning Promises is why
calling await on a promise directly vs calling it on a variable containing a
promise is different. Of course the root cause of the misunderstanding is not
understanding Promises are executed immediately and await blocks on a promise.
So creating a promise after an await causes the second promise to be executed
after the awaited promise is resolved.

For me I still had my mental model from a lazy execution environment (Dask)
and that’s where my misconception came from.

~~~
akiselev
It's a common problem with many languages - I've experienced it recently
myself with Rust's futures. It requires you to be explicit about spawning
tasks since that requires a heap allocation and it took me a while to figure
out that I had to explicitly include an async runtime (of which there are
several) and start up the async tasks myself.

That said, I agree with the GP. I immediately saw that it was going to execute
sequentially and thought nothing of it. Most of the time I use async/await not
for concurrency but for syntax sugar and to get proper exceptions from promise
based APIs instead of the GOTO FAIL control flow breaking nonsense that is
catch().

~~~
steveklabnik
It doesn't _inherently_ require a heap allocation, but the common, non-
embedded use case does.

~~~
akiselev
Yeah I was glossing over the specifics for this context. The way
futures/generators are implemented as syntax sugar over a closure/state
machine that can live on the stack is one of my favorite features of Rust, but
i.e. an executor that uses a smallvec task queue is a bit esoteric for a
discussion that started around Javascript.

------
Etheryte
While the idea is correct, the proposed solution is needlessly verbose. A
better solution would be to

    
    
      async function getPeople() {
        const [members, nonMembers] = await Promise.all([
          fetch("/members"),
          fetch("/non-members"),
        ]);
        return members.concat(nonMembers);
      }
    

The gist of the article is very true though. This is something that comes up
every now and then in code review, even with seasoned developers. I'm not
entirely sure what the solution would be since depending on the context, both
sequential and parallel execution can be correct. It might just be one of
those rite of passage type of mistakes you have to make once or twice.

~~~
bryanlarsen
or

    
    
        async function getPeople() {
          const membersPromise=fetch("/members");
          const nonmembersPromise=fetch("/non-members");
          return [].concat(await membersPromise, await nonmembersPromise);
        }
    

edited to fix typo spotted by lhorie. thanks.

~~~
d1plo1d
I expect that will execute sequentially though as opposed to the solution
above it.

~~~
TheNorthman
Judging from your comment history, are you perhaps used to writing
asynchronous code in Rust? In most languages, `Futures` start executing from
when they are spawned. This is opposed to Rust, where they execute when they
are `.await`ed.

~~~
d1plo1d
Ok, 3rd edit. The promises are awaited in sequence but since they are both
started before the first await they are run in parallel.

Your question about Rust futures makes a ton more sense now. This only works
because a promise is self-executing / not inert like a Rust Future. Thank you
kind stranger! Your async-foo is strong ^_^

------
andyjpb
I'm not a JS programmer so the intended "simultaneous" waits only vaguely
registered as possibly problematic.

However, after reading the article it is, at worst, a performance problem.

...and performance should come after the code is correct and I don't think the
code is even semantically correct in the first place.

I'm assuming that "members" and "non-members" are distinct subsets of all
People. i.e. it's not possible to be in the "members" and "non-members" sets
at the same time. Additionally, everyone is in one or the other.

Because the set membership is queried with 2 independent queries (designed in
a reasonable and common RESTful manner) there is a race condition where an
object (that changes in the time between the two calls) might appear in
neither or both of the sets.

These results are then blindly concatenated together.

If the API wants to retain its RESTful design, it must include metadata about
the consistency between calls. For example some kind of token that can be
compared to check that the replies to the two queries represent a consistent
view of the data.

If the tokens, differ, the "transaction" can be retried.

Alternatively, the API can be designed, possibly making it less RESTful in the
process, so that the transaction is implemented on the server.

There are many other ways to guarantee a correct and consistent result.

Either change has big enough implications that optimising for performance at
this stage would be premature because of the amount of refactoring necessary
to implement `getPeople` in a way that provides consistent and correct
semantics.

~~~
brundolf
You make a valid point, which I probably should have mentioned, that this
doesn't apply for stateful calls. But in my experience it's a common use-case
(at least on the front-end) to grab two or more independent datasets, from
stateless endpoints, at the same time, for display purposes. And putting those
into a sequence can have a dramatic impact on perceived performance. I don't
think a performance factor of 2x or more is a micro-optimization that one
should hold off on making until a later pass.

------
eat_veggies
Sometimes (most of the time?) you want to run two async functions in series
because the input of one function depends on the output of the other, or
because you want the side effects of each to happen in the correct order. This
is the whole point of promise chaining and the async/await sugar on top of it.
Concurrency is hard, and I am glad that the default is something predictable
but slow, rather than fast and racy.

~~~
enedil
Except the default is exactly fast and racy:

fetch(x); fetch(y);

vs

await fetch(x); await fetch(y);

------
uDontKnowMe
Would this not also work?

    
    
        async function getPeople() {
          const members = fetch("/members");
          const nonMembers = fetch("/non-members");
        
          return (await members).concat(await nonMembers);
        }

~~~
brundolf
Interesting, it actually does. I agree that's cleaner than the Promise.all()
pattern.

------
onion2k
I hate that await has made it easy to turn JS in to a synchronous language. I
see so much front end code that would be better for users without await, but
lots of developers prefer to write code that's easier to think about rather
than fast. It's hugely annoying.

~~~
rectang
But very little code has to be fast, right? Best to prioritize making code
"easier to think about" and thus more likely _correct_ , then to profile and
optimize the bottlenecks.

~~~
nine_k
All the code in the UI should better be fast, else the UI drowns in annoying
molasses real quick.

So no, performance does matter a lot here.

~~~
rectang
I don't dispute that UI code needs to be fast overall, but it's still the case
that only a small proportion of the code needs to be fast in order for the
whole thing to be fast.

FWIW I worked in the search/database space for a decade and it's the same
thing there. There was one time I wrote a lock-free hash table implementation
for a registry that needed to be optimized for concurrent access; that code
was much harder to write and to understand than the rest of the codebase, but
it was justified. Most of the codebase, though, was not performance-critical.

EDIT: I will grant that you still need to think about how to architect the
project so that it is optimizable later, which can mean thinking about what
needs to be async early on.

------
brundolf
Author here: lots of people are weighing in saying this is a micro-
optimization and/or an uncommon case (making independent, stateless requests
in parallel). My suspicion is that these people are not front-end programmers
(a factor that I hadn't considered as making a difference, when writing this
post).

On the server, maybe having a set of requests take two seconds instead of one
second (without consuming extra CPU cycles) doesn't really matter. On the
front-end, it matters a whole lot. People go to much greater lengths to shave
off much smaller amounts of time. Otherwise people on orange websites complain
about your site being slow ;)

On the server, maybe it's a really common case to be making requests that
modify state, and/or to be stitching together several microservices that
require IDs or other information to be passed between them. On the front-end,
especially on first-load, you're often loading up several different datasets
purely for display purposes. These are rarely written to depend on each other,
because of the latency issue in my previous point.

It is interesting though to see that there's a difference in norms/use-cases
between the two.

~~~
TheCoelacanth
I don't think it's so much that a second is more significant on the front-end.
An entire second is practically an eternity on the backend.

The main difference is how much latency a typical request is likely to have.

On the backend, this optimization would frequently only save you 50 ms,
because you are making a low latency call to another service that you are
running in the same datacenter.

~~~
brundolf
That's a good point. And I guess the payloads will often be smaller, because
the large datasets often just stay in the DB until needed. Whereas if the
front-end needs to load data for a huge table (from a location that's
obviously not in the same datacenter), it's easy for this optimization to save
you hundreds of milliseconds if not entire seconds.

------
shauns
I think people miss the wait part of await and hence forget they can separate
the creation and consumption of a promise. Should have been called waitFor!

~~~
happytoexplain
As a non-JS person, I thought it was obvious what await does, but now I'm
second guessing myself. What does it do that's not the "wait" part? Is it not
simply causing a normally asynchronous function to block?

Edit: Sorry for the late edit, but: Is it because users are using it thinking
it just "unwraps" a promise into its resulting value? I can begin to
understand that, but it seems so incidental to the primary concept of
"waiting" for something asynchronous to return.

------
spankalee
I thought this was going to be able stale data and race conditions, which is a
much more insidious problem, IMO. The trifecta of mutability, shared state,
and async operations can cause some very hard to reason about bugs.

------
fungifam
Clickbait title. Beware of not reading the docs and making noob mistakes.

------
recursive
The way I would write the code is never covered, and I can't tell it's because
I don't understand async, or I'm secretly a genius.

The article says this is correct.

> const both = await Promise.all([ members, nonMembers ]); > return
> both[0].concat(both[1]);

I would have written it this way without the third promise.

> return (await members).concat(await nonMembers);

Is this wrong somehow?

~~~
nine_k
This is not wrong, it's just not parallel. That is, the second promise, inside
`.concat()`, will only get created and scheduled _after_ the `await members`
will have resolved.

The point of `Promise.all` is to have every promise passed to it to start
computing concurrently, likely in parallel if they e.g. start independent I/O
operations.

~~~
ctidd
If I follow your statement, it doesn't appear correct. JS Promises are eager,
and the work underlying the promise will begin executing prior to being
awaited. Await is just blocking on that execution's resolution.

    
    
        // Given:
        const fooPromise = asyncFoo();
        const barPromise = asyncBar();
    
        // Then this...
        await fooPromise;
        await barPromise;
    
        // ...is equivalent to this:
        await Promise.all([fooPromise, barPromise]);
    

Now, Promise.all() is useful, but it is unrelated to when computation starts.

~~~
nine_k
I can't get it, and from my experience, this is not true.

Consider:

    
    
      const x = await foo();
      await bar(x);
    

How can you schedule `foo()` and `bar()` in parallel, when they have an
explicit data dependency?

~~~
recursive
> How can you schedule `foo()` and `bar()` in parallel, when they have an
> explicit data dependency?

You can't, but that's a fundamentally different scenario than what's being
discussed.

------
azangru
> Can you spot the problem with this piece of code?

Sure. There's no error handling :-)

(Also, if it's not some magic fetch, the return value needs to be json-ed.)

~~~
wereHamster
There doesn't need to be, if you expect the caller (or the top-level code) to
handle errors.

Also, how should the function handle errors? It's not immediately obvious. An
empty array is not the correct thing to return (an empty array could be a
valid return value). Returning undefined/null would be another option, but
that hides the error from the caller (which might be interested in the exact
error message, to be able to properly deal with it).

~~~
jakear
Simple!

try { ... } catch (e) { throw “an unknown error occurred” }

;)

------
xixixao
Hack has a more readable "fix" now, which will hopefully make it to JS in due
course:

    
    
      async function getPeople() {
        concurrent {
          const members = await fetch("/members");
          const nonMembers = await fetch("/non-members");
        }
    
        return members.concat(nonMembers);
      }

~~~
jakear
...why? Please, please, don’t add syntax to “fix” semantics that can already
be elegantly expressed using existing constructs (Promise.all). Minimal-ish
syntax is one of the few areas JS gets things right.

~~~
xixixao
Your argument can be applied against await/async in general - it doesn’t allow
anything Promises didn’t allow before.

The reason is that it’s easy to make a mistake when destructuring the
Promise.all call, and the Promise.all call doesn’t scale well (here dummy
example, spot the bug)

    
    
      const [foo, baz, bar] = Promise.all([getFoo(), getBar(), getBaz()])
    

This is much more readable and less error prone:

    
    
      concurrent {
        const foo = await getFoo();
        const bar = await getBar();
        const baz = await getBaz();
      }

~~~
jakear
There’s always a line and it must be drawn somewhere. Async/await allow for
chaining deferred execution blocks without introducing deep syntactic nesting,
at the cost of 2 keywords. In my opinion the benefits of flattening the
language outweigh the costs of adding the keywords.

The hack approach otoh introduces an entirely new syntax construct (tagged
blocks), which declares variables that escape their initialization block
(unexpected in the JS world post-let/const), and strongly leverages the
existing hack execution model whereby all await’s in a single statement are
executed concurrently. This execution model is incompatible with the rest of
JS (hack disallows many styles of having multiple awaits in an expression,
such as `await a && await b`), and at the end of the day what are the
benefits? It executes a series of statements in parallel, exactly the same as
Promise.all.

If you’re concerned about mixing and matching declarations, maybe try

let foo, bar, baz;

await Promise.all([

    
    
       foo = getFoo(),
    
       bar = getBar(),
    
       baz = getBaz()])
    

?

Edit: realized in the above example the variables remain of promise type,
which isn’t great. One could alternatively do “getFoo().then(result => foo =
result)”, but I agree this isn’t great either.

------
torartc
Why is this article so highly upvoted? The problem clearly isn't with
Async/Await itself.

~~~
happytoexplain
This is what's confusing me - I have no problem with the complaint that a
language feature is confusing or unfriendly to beginners, but in this
particular case, why would somebody who doesn't know what await does even use
it? What are they expecting it to do besides wait?

------
asimpletune
I’ll admit that I don’t know a lot about JavaScript, but it’s a bit surreal
coming from a Scala background and reading this blog. Like, I literally
couldn’t understand how someone could make this mistake, until I remembered
that there’re no types in JS. On that same note, Rust takes things even one
step further and adds a “lifetime” axis, which if you’re used to then a
similar blog post about memory access issues would be met with equal confusion
about the author’s confusion. Hope that made sense.

------
abhishekjha
You are not supposed to await immediately on the async tasks. Let them get
fired in parallel without the await. Apply await only when you need to see the
resolve/reject results.

------
tpetry
The interesting point about this example is that an intelligent js compiler
could easily rewrite this code into parallel execution. Its possible because
the low-level await statement is declaring to wait on the execution but the
calculated value is used much later first.

Maybe someone could built this for typescript, the strict typesystem should
make this easily (?) solvable by some hidden wrapper type, like
AwaitedButUnaccesedPromise.

~~~
Etheryte
The problem is it may be intentional to have the requests be sequential
depending on what you're doing. It would be straightforward to identify this
kind of code, but the compiler wouldn't be able to tell which was the correct
business logic.

~~~
brundolf
Yes, as another commenter pointed out, this would get you into trouble if the
endpoint you're calling is stateful at all. And that's definitely not
something a compiler could ever reason about.

~~~
tpetry
Oh thats true, didnt think about it. But this problem could be easily solved
by adding a „defer“ keyword which is basically await under the hood with the
added compiler optimizations.

------
tobyhinloopen
I thought it was about lack of error handling.

Running them in parallel is not a solution. I would create a new API call to
fetch both.

------
_greim_
Of the potential footguns with async/await, I'd rate this as low to non-
existent. This falls into the category of straightforward code which gets a
task done and which hasn't been optimized to within an inch of its life.

------
justsomeuser
Another issue: you could end up with duplicate people in the set if they are
moved ‘member <-> !member‘ between each read.

What you would need is a single HTTP request that uses a DB read transaction
to get the whole set of people.

------
gamell
I wouldn't call it necessarily a "mistake" but certainly something you would
want to avoid doing if you want your code to be performant.

------
fastball
I actually like making this kinda mistake, because it means I can get some
easy performance gains in a bugfix release!

------
Smaug123
The clue is even in the name. What do you expect to do on the keyword "await"
except stop and wait?

------
gamesbrainiac
I miss the defer keyword in coffeescript.

------
rough-sea
The mistake is not getting the body of the response. Is it even meaningful to
concat two response objects.

It's amazing what gets upvoted on HN.

