8 ms·

Mistakes we make using JavaScript Promises

• borschtplease 7y ago
  And he makes mistake #1 in mistake #3 by running „get“ sequentially instead of using Promise.all
  • jchw 7y ago
    They're all artificial examples: one could assume that the tasks in #3 are intended to be executed serially. Though there aren't any data dependencies expressed between the different calls, perhaps ordering could be important for other reasons.
    • TheCycoONE 7y ago
      I feel it should have been called out explicitly when introducing await, because the 'clean' solution in async/await code is to call each async function and then await the results where you need them - which is a pattern he doesn't hint at at all.
      • jchw 7y ago
        I think there is a danger in that approach: if you forget to await, errors are silently ignored. You can also await a Promise.all, which is reasonably good enough if you do depend on all of the results to do anything meaningful anyways.
        • TheCycoONE 7y ago
          If you need you can name your intermediate results fooPromise or something else to make it obvious that it isn't meant to be used directly. The async article linked from this article has a toy example for this concept where the result is unused, but most of the time you would be passing the result somewhere. If you were using typescript it could fail at compile time when you try to pass a promise instead of the expected type. Even if you're not, you should notice it never work in your tests.
          • jchw 7y ago
            Sorry, by forget to await, I mean you call an async function but then don’t ever actually use the result. TypeScript can also diagnose that one by illuminating unused variables. Still, I think it is easiest to never mess up if you await as soon as you have a promise value. In many common cases this is easy enough.
        • twooster 7y ago
          A bigger danger still would be: const userPromise = fetchUser(id) const itemPromise = fetchItem(itemId) // Do stuff, maybe even more async stuff const item = await itemPromise const user = await userPromise Since those promises haven't been awaited until later in the code, they could throw and result in an unhandledRejection, which would be pretty bad. Promise.all is much safer since it instantly awaits both promises.
• CraftThatBlock 7y ago
  Are these "common" mistakes? With basic understanding of Promises, these mistakes shouldn't happen. Also I believe #1 is wrong when using async/await.
  • tehlike 7y ago
    Some older libraries will still be callback driven, and one important thing is always resolving or rejecting promise.
    • CraftThatBlock 7y ago
      If it's callback driven, is it a Promise mistake or a callback mistake? I don't think it would apply here
      • tehlike 7y ago
        Most of the time first thing i do is to wrap the library call in a promise. Callbacks are really bad after a few nested ones, bht promise doesnt eliminate all the problrms
  • jonny_eh 7y ago
    > With basic understanding of Promises, these mistakes shouldn't happen That's why blog posts like this exist, to teach people the basics. In the real world people often start using technology before they fully understand it, and then get sent blog posts like this to read (or catch them on HN).
• franciscop 7y ago
  This "solution" given here has a syntax error since you don't have access to `user` within the second then() callback: get("http://data.com/user") .then(user => get("http://data.com/location" + user.id)) .then(location => createEntry(user, location)) .then(response => { // handle response }).catch(err => { // handle failure }); Instead, back before async/await made things easier this nested pattern was used (notice the difference in parenthesis location): get("http://data.com/user") .then(user => get("http://data.com/location" + user.id) .then(location => createEntry(user, location))) .then(response => { // handle response }).catch(err => { // handle failure }); Edit for completeness, this is how it is now with async/await: try { const user = await get("http://data.com/user"); const location = await get("http://data.com/location" + user.id); const response = await createEntry(user, location); // handle response } catch (error) { // handle failure }
  • fooey 7y ago
    Instead of subtle nesting, I would have passed through the user as part of the result of the second promise. get('//data.com/user) .then(user => resolve({user, location: get('//data.com/user')})) .then(({user, location}) => createEntry(user, location)) .then(response => { // handle response }).catch(err => { // handle failure }); Async/await is much cleaner now though.
    • korla 7y ago
      How does the resolve function work here? It is passed an object where one property is an object and the other a promise. The next then suddenly receives the resolved location..?
      • deleted 7y ago
        [deleted]
      • hoorayimhelping 7y ago
        The argument passed to `resolve` gets returned from the resolve function and becomes available to any handler that handles it with `then`. In the above example, the object is being destructured as part of the arguments. The destructuring is probably what's tripping you up. // straightforward, no magic const simpleFunction = function() { return Promise.resolve({ name: 'simpleObject' }); } simpleFunction().then(function(response) { console.log(response); // { name: 'simpleObject' } }); // alternatively, destructuring the arguments simpleFunction().then(function({ name }) { console.log(name); // 'simpleObject' });
• deleted 7y ago
  [deleted]
• betamark 7y ago
  Thanks for all the feedback! I will correct the article shortly
• ilaksh 7y ago
  Metamask doesn't like the domain: "Ethereum Phishing Detection" This domain is currently on the MetaMask domain warning list
• hayksaakian 7y ago
  Hopefully with async/await we can all put this behind us. In my own projects, async/await has made improved readbility and reduced errors.
  • gatherhunterer 7y ago
    I have found cases in which async/await won’t cut it quite as cleanly as “return new Promise”. AWS Cognito’s API has functions that take an object parameter with properties onSuccess and onFailure. If these functions are being called with programmatically created values within promises you can use “await new Promise” but that means declaring an additional async/await function (and therefore a promise) that resolves with the resolution of another promise. When you are explicitly defining all of your operations in TypeScript you notice these anti-patterns. Of course you could also declare these functions elsewhere and use function parameters instead of scoped variables. In my opinion the best solution is the one that is most justifiable when described to an outsider while walking through the code so I sometimes feel grateful for the freedom to choose my promise implementations on a case-by-case basis. When the relevant code is entirely my own, however, async/await is the most elegant. Async/await is pure syntactic sugar and should not reduce errors unless the Promise syntax was implemented incorrectly.
    • james-mcelwain 7y ago
      I'm not sure I fully understand. Interoperability is as simple as wrapping the non-standard API, no? This way you have a standard calling style in your code. It's not a weakness of async/await that some libraries don't have promise friendly APIs.
      • gatherhunterer 7y ago
        The API in this example is promise-friendly, it is just intended for callback syntax; providing two situational callback options. This an implementation issue. Of course you can wrap the non-conformant external code but in this case that means accepting anti-patterns in order to gain the benefit of syntactic sugar. Writing API wrappers that do nothing more than wrap traditional Promise callback code as a rule is a poor substitute for allowing developers to choose the most appropriate implementation. Instead that would favor meaningless promise chaining and unless you ignore a library’s type definitions that is pretty hard to miss. Using async/await isn’t doing anything different in the same sense that using a class is no different than using a function that returns an object. The question is one of readability and flexibility, no one syntax is inherently better that the other because the interpreter does not know the difference.
• acjohnson55 7y ago
  One thing I realized about async/await is that it removes the ability to use the synchronous continuation of an async function call. In return, it makes its asynchronous continuation feel synchronous. Technically, that's less power, but I realized that's almost always a good thing.
  • hn_throwaway_99 7y ago
    I don't understand what you're saying here. An async function just returns a Promise, that's it. You don't have to await on it immediately - you could just as easily assign it to a variable and await on it some time later.
    • randallsquared 7y ago
      ...or call .then() on it.
    • acjohnson55 7y ago
      I should really say `await`, in particular. But obviously, you can't `await` outside of an `async` function (proposed top-level `await` notwithstanding). It's true that you'll eventually get to the top of the `async` function stack and you'll have an actual Promise value, at which point, you're back in synchronous continuation land. But point is that using `async`/`await` as much as possible restricts the programmer to less concurrency, which inevitably means fewer glitches and race conditions. Many people make the mistake of thinking that JavaScript is safe because there's only one thread. But it turns out that most of the hazards of concurrency still exist as long as continuations can interleave with access to shared resources and mutable state. Does that make more sense? It's late over here, so I might not be super clear.
• hn_throwaway_99 7y ago
  With respect to Promise.all: > In this example, both promises will be processed asynchronously and only when both of them are resolved, we handle the result. While that is true if all the promises resolve successfully, if any of the promises get rejected the catch block gets executed as soon as the first rejection occurs. Beware of that behavior, as if you're not aware of it it can leave your system in a bad state (speaking from experience, of course :)
• seniorsassycat 7y ago
  _any_ function that returns a promise should have the `async` modifier - even if `await` isn't used. A function may return a promise or throw an exception. An `async` function may only return a promise. async functions cannot throw exceptions. ``` // This code sucks but you might have to write it if `get` isn't an async function. try { get().catch(_ => /* handle async errors /) } catch { / handle sync errors */ } ```
  • majewsky 7y ago
    HN does not support the Markdown syntax for code blocks. You need to indent all lines by two spaces to get a code block.
  • matt-attack 7y ago
    Interesting about adding async. I actually didn’t know it eliminated the possibility of it throwing an error. What happens if it explicitly throws an error?
    • seniorsassycat 7y ago
      An async function that throws and error will return a promise that will reject with the thrown error.
• NohatCoder 7y ago
  Lots of bikeshedding about minute differences in syntax. Especially #3 rubs me the wrong way, it is all syntactic sugar, the only tangible thing you get from all this is to prevent the code from working in older browsers.
• qazpot 7y ago
  I find Promises to be nothing but fancy looking syntax sugar over the callbacks. Infact, using the Async library one can write more cleaner and easy to understand code using callbacks as compared to Promises.