Re: Consider avoiding allSettled in tests (was: Re: Intent to ship: Promise.allSettled)

2019-11-01 Thread Kris Maglione
On Thu, Oct 31, 2019, 06:02 Jason Orendorff  wrote:

> Ignoring the awaited value here is like using `catch {}` to squelch all
> exceptions, or ignoring the return value of an async function or method, or
> any other expression that produces a Promise. Do we have lints for those
> pitfalls? I'm kind of surprised that there doesn't seem to be a built-in
> ESLint lint for `catch {}`.
>

Some directories have an ESLint rule to disallow empty blocks, which
requires that any empty catch blocks at least contain a comment to justify
them.

>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Consider avoiding allSettled in tests

2019-11-01 Thread Paolo Amadini

On 10/31/2019 1:02 PM, Jason Orendorff wrote:

On Thu, Oct 31, 2019 at 4:10 AM Paolo Amadini 
wrote:


// INCORRECT
//await Promise.allSettled([promise1, promise2]);

The last example silently loses any rejection state.



Ignoring the awaited value here is like using `catch {}` to squelch all
exceptions, or ignoring the return value of an async function or method, or
any other expression that produces a Promise. Do we have lints for those
pitfalls? I'm kind of surprised that there doesn't seem to be a built-in
ESLint lint for `catch {}`.


Correct, what makes this example worth noticing is that it looks very
similar to other valid uses where we can ignore the resolution values,
because they are "undefined" or irrelevant to the flow.


There's nothing special about tests here, right? Those patterns are fishy
whether you're in a test or not.


I made the point with tests because in many cases, when racing promises
there, every failure should be fatal, and at the same time we often
legitimately drop return values that are irrelevant to the test.

Cheers,
Paolo
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Consider avoiding allSettled in tests (was: Re: Intent to ship: Promise.allSettled)

2019-10-31 Thread Jason Orendorff
On Thu, Oct 31, 2019 at 4:10 AM Paolo Amadini 
wrote:

>// INCORRECT
>//await Promise.allSettled([promise1, promise2]);
>
> The last example silently loses any rejection state.
>

Ignoring the awaited value here is like using `catch {}` to squelch all
exceptions, or ignoring the return value of an async function or method, or
any other expression that produces a Promise. Do we have lints for those
pitfalls? I'm kind of surprised that there doesn't seem to be a built-in
ESLint lint for `catch {}`.

There's nothing special about tests here, right? Those patterns are fishy
whether you're in a test or not.

-j
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform