pooledMap can throw uncaught `undefined` when cancelled

[danopia]

Describe the bug

If pooledMap gets canceled, it seems to throw an uncaught undefined value. Cancelling can happen due to returning from within the loop body. The undefined does not seem to be catchable with a try {} catch {} construction.

The lack of a stack trace can make debugging tricky, as the developer may not immediately suspect pooledMap to be the source of the issue.

Steps to Reproduce

import { pooledMap } from "https://deno.land/[email protected]/async/pool.ts";

Deno.test('cancel pooledMap', async () => {
  async function firstNumber() {
    for await (const num of pooledMap(4, [100], num => Promise.resolve(num))) {
      return num;
    }
  }
  await firstNumber();
});

> deno test
running 1 test from pooledMap_test.ts
test cancel pooledMap ...
test result: FAILED. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (34ms)

error: Uncaught (in promise) undefined

Expected behavior

In this specific case, there were no remaining items to map over, so I expected pooledMap to cleanly accept the cancelation. I had a routine where some outputs result in immediately returning a specific value, so it's a bit cleaner to return from the loop body.

Alternatively, if cancellation isn't considered sound, I'd expect pooledMap to throw an actual Error so that a stack trace is printed.

I'd also expect any error to bubbled through my calling code instead of being uncaught.

Extra information

If poolLimit is set to 1 (disabling concurrency), then an actual error is thrown. It is still uncaught:

import { pooledMap } from "https://deno.land/[email protected]/async/pool.ts";

Deno.test('cancel pooledMap', async () => {
  async function firstNumber() {
    for await (const num of pooledMap(1, [100], num => Promise.resolve(num))) {
      return num;
    }
  }
  try {
    await firstNumber();
  } catch (err) {
    // attempt to not crash the program
    console.log('err:', err);
  }
});

> deno test
running 1 test from pooledMap_test.ts
test cancel pooledMap ...
test result: FAILED. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (26ms)

error: Uncaught (in promise) TypeError: Writable stream is closed or errored.
      writer.close();
             ^
    at writableStreamClose (deno:ext/web/06_streams.js:3541:9)
    at writableStreamDefaultWriterClose (deno:ext/web/06_streams.js:3778:12)
    at WritableStreamDefaultWriter.close (deno:ext/web/06_streams.js:5534:14)
    at https://deno.land/[email protected]/async/pool.ts:54:14

Environment

• OS: Debian GNU/Linux 11 (bullseye)
• deno version: 1.20.5
• std version: Found on 0.130.0, verified on 0.138.0

[lino-levan]

A quick update for this one. This still errors out in an uncatchable fashion, but the error message for the first case makes it a bit more clear:

running 1 test from ./test.ts
cancel pooledMap ...
Uncaught error from ./test.ts FAILED
cancel pooledMap ... cancelled (0ms)

 ERRORS 

./test.ts (uncaught error)
error: (in promise) undefined
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

deno: v1.27.0 std: v0.161.0 hardware: m1 macbook air