Invalid state: Controller is already closed

[FZambia]

Bug Description

When running my tests with undici v5.8.0 I am getting the traceback:

node:internal/errors:466
    ErrorCaptureStackTrace(err);
    ^

TypeError: Invalid state: Controller is already closed
    at new NodeError (node:internal/errors:377:5)
    at ReadableStreamDefaultController.enqueue (node:internal/webstreams/readablestream:992:13)
    at Fetch.fetchParams.controller.resume (/Users/fz/projects/centrifugal/centrifuge-js/node_modules/undici/lib/fetch/index.js:1864:41)
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'ERR_INVALID_STATE'

I did some research if this can be helpful:

In my case the code in undici is getting exception in calling await fetchParams.controller.next() in lib/fetch/index.js, so catch branch works:

      try {
        const { done, value } = await fetchParams.controller.next()

        if (isAborted(fetchParams)) {
          break
        }

        bytes = done ? undefined : value
      } catch (err) {
        if (fetchParams.controller.ended && !timingInfo.encodedBodySize) {
          // zlib doesn't like empty streams.
          bytes = undefined
        } else {
          bytes = err
        }
      }

I added some console logging:

      } catch (err) {
        console.log(err); // DOMException [AbortError]: The operation was aborted.
        console.log(fetchParams.controller.ended); // undefined
        console.log(timingInfo.encodedBodySize); // 136
        console.log(fetchParams.controller.ended && !timingInfo.encodedBodySize); // undefined
        if (fetchParams.controller.ended && !timingInfo.encodedBodySize) {
          // zlib doesn't like empty streams.
          bytes = undefined
        } else {
          console.log("bytes is err"); // bytes is err
          bytes = err
          console.log(bytes instanceof Error); // false
          console.log(typeof bytes); // object
        }
      }

The detailed exception is:

DOMException [AbortError]: The operation was aborted.
        at new DOMException (node:internal/per_context/domexception:51:5)
        at Fetch.abort (/Users/fz/projects/centrifugal/centrifuge-js/node_modules/undici/lib/fetch/index.js:89:20)
        at AbortSignal.requestObject.signal.addEventListener.once (/Users/fz/projects/centrifugal/centrifuge-js/node_modules/undici/lib/fetch/index.js:165:20)
        at AbortSignal.[nodejs.internal.kHybridDispatch] (node:internal/event_target:639:20)
        at AbortSignal.dispatchEvent (node:internal/event_target:581:26)
        at abortSignal (node:internal/abort_controller:291:10)
        at AbortController.abort (node:internal/abort_controller:321:5)
        at AbortSignal.abort (/Users/fz/projects/centrifugal/centrifuge-js/node_modules/undici/lib/fetch/request.js:372:32)
        at AbortSignal.[nodejs.internal.kHybridDispatch] (node:internal/event_target:639:20)
        at AbortSignal.dispatchEvent (node:internal/event_target:581:26)

So I suppose fetchParams.controller.controller.enqueue(new Uint8Array(bytes)) is called even if the stream was aborted.

Reproducible By

Create a new Node project with npm init

Add jest:

npm install --save-dev jest

Add test file index.test.js:

test('test exception', async () => {
    const d = new DOMException('test')
    expect(d).toBeInstanceOf(Error)
    expect(d instanceof Error).toBeTruthy()
})

Update package.json to have:

{
  "scripts": {
    "test": "jest"
  }
}

Run npm test

Expected Behavior

DOMException in Jest env does not extend Error, so stream aborting logic in https://github.com/nodejs/undici/blob/26f60b7b6e612bb831133d7f85914963d1955011/lib/fetch/index.js#L1857 does not work.

Environment

Node v18.2.0

Running tests with jest

Additional context

This reproduces starting from undici v5.6.0

[mcollina]

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

[FZambia]

Yep, I'll try to provide an example, not very simple to create a minimal one.

Think I found the PR which caused an issue in my case: https://github.com/nodejs/undici/pull/1511

[FZambia]

So now I see that the reason is that when running tests with Jest:

test('test exception', async () => {
  console.log(globalThis.DOMException);
  const d = new globalThis.DOMException("");
  console.log(d instanceof Error);
})

DOMException instance is not an instance of Error, so https://github.com/nodejs/undici/blob/26f60b7b6e612bb831133d7f85914963d1955011/lib/fetch/index.js#L1857

is not true.

But when simply running the same code with Node DOMException is an instance of Error. Previously (before undici v5.6.0 and https://github.com/nodejs/undici/pull/1511 in particular) there was custom AbortError that extended Error so my tests worked fine.

Do not understand at the moment whether this is Jest problem or Undici's, or my own, and why DOMException is not an instance of Error in Jest case.

[ronag]

We could make a isErrorLike helper which is a bit more forgiving. We've done this in other cases for increased interop.

[KhafraDev]

This is an undici issue caused by #1511.

We still need a small repro for tests.

[FZambia]

DOMException doesn't extend Error, even on the browser.

I tried in Chrome and Firefox:

> const d = new DOMException()
undefined
> d instanceof Error
true

Seems it's an instance of Error, or I am missing sth?

[KhafraDev]

No, I was mistaken. However this seems more like a jest issue than an undici issue.

edit: If possible, you could try using vitest, which includes a similar api to jest. I've seen some projects move away from jest and it seems like a good alternative.

import { test, expect } from 'vitest' 

test('test exception', async () => {
  const d = new DOMException('')
  expect(d).toBeInstanceOf(Error)
  expect(d instanceof Error).toBeTruthy()
})

[FZambia]

edit: If possible, you could try using vitest, which includes a similar api to jest. I've seen some projects move away from jest and it seems like a good alternative.

Thanks for a suggestion, jest has a nice --detectOpenHandles option which is really important for my use case, seems that vitest does not have it yet, so not sure I want to switch.

I don't see a workaround from my side for this, I can open an issue in Jest repo if you think it should be fixed there and link it to this one, or possibly you see a good way (like the one @ronag suggested above or handling DomException explicitly) to fix this from Undici's side?

[KhafraDev]

Jest has had an issue with instanceof Error for over half a decade now, this will most likely need to be fixed here. I'll work on this tomorrow.

[FZambia]

Many thanks for helping with this, I tried to run tests with undici from the latest commit and it worked fine.

[aralroca]

I have the same problem with Node 20. Switching to Node 22 is working fine, however my cloud provider is using the LTS (20) and I can't move to 22 there. Any workaround to solve it?

[KhafraDev]

Stop using jest or use undici rather than the globals. Jest is bad.