Cache interceptor failing with "invalid onError method"

[callumgare]

Bug Description

I'm trying to make use of Undici 7's new caching feature but I'm getting this error when trying to follow the example provided in the announcement blog post: https://blog.platformatic.dev/undici-v7-is-here#heading-caching

node:internal/deps/undici/undici:591
            throw new InvalidArgumentError("invalid onError method");
                  ^

InvalidArgumentError: invalid onError method
    at Agent.dispatch (node:internal/deps/undici/undici:591:19)
    at handleUncachedResponse (/private/tmp/undici-test/node_modules/undici/lib/interceptor/cache.js:102:10)
    at handleResult (/private/tmp/undici-test/node_modules/undici/lib/interceptor/cache.js:205:12)
    at ComposedDispatcher.<anonymous> (/private/tmp/undici-test/node_modules/undici/lib/interceptor/cache.js:348:9)
    at ComposedDispatcher.dispatch (node:internal/deps/undici/undici:425:23)
    at ComposedDispatcher.request (/private/tmp/undici-test/node_modules/undici/lib/api/api-request.js:188:10)
    at /private/tmp/undici-test/node_modules/undici/lib/api/api-request.js:179:15
    at new Promise (<anonymous>)
    at ComposedDispatcher.request (/private/tmp/undici-test/node_modules/undici/lib/api/api-request.js:178:12)
    at /private/tmp/undici-test/node_modules/undici/index.js:102:15 {
  code: 'UND_ERR_INVALID_ARG'
}

Reproducible By

import { setGlobalDispatcher, getGlobalDispatcher, interceptors, request } from 'undici'
import {createServer} from "node:http"

/* Create simple http server that returns timestamp */
const server = createServer((req, res) => {
  res.setHeader('Cache-Control', 'public, max-age=604800, immutable');
  // res.setHeader('Cache-Control', 'no-store');
  res.end(Date.now().toString())
}).listen(0)
const address = server.address()
if (!address || typeof address === "string") {
  throw Error("Failed to start server")
}
const url = `http://localhost:${address.port}`

/* Set global dispatcher to use caching */
setGlobalDispatcher(getGlobalDispatcher().compose(
  interceptors.cache())
)

let res

res = await request(url)
console.log(await res.text(), {cache: "force-cache"})

res = await fetch(url)
// This should be cached and thus return the same timestamp as the previous request
console.log(await res.text(), {cache: "force-cache"})

I'm using Node.js v23.4.0 (and also tried v23.5.0 with the same result) on macOS Sonoma 14.5. I've tried undici version 7.2.0, 7.1.1, 7.1.0 and 7.0.0.

Expected Behavior

No error to be thrown and the same response body to be printed for both requests.

Additional context

I also tried this example using the SqliteCacheStore but get the same error message. Although it did create a "cache.db" file.

[metcoder95]

This is somehow expected; the dispatcher returned by getGlobalDispatcher is not compatible with the latest changes made to the interceptors.

If changed from

setGlobalDispatcher(getGlobalDispatcher().compose(interceptors.cache()))

to

setGlobalDispatcher(new Agent().compose(interceptors.cache()))

The issue is gone; it seems we sadly broke the contract here without noticing.

[mcollina]

@metcoder95 the two behaviors should ideally be identitical in the given snippet, so something very odd is at play.

My theory is that somehow undic / fetch gets loaded in Node.js core before the one on the first line. Anyhow, we should be able to handle this.

@ronag wdyt? Should we bump the global undici version and deal with this somewhat?

[ronag]

@ronag wdyt? Should we bump the global undici version and deal with this somewhat?

Sure but I'm not sure how we deal with it?

Should we just revert the API changes and find a less breaking variation?

[metcoder95]

My theory is that somehow undic / fetch gets loaded in Node.js core before the one on the first line. Anyhow, we should be able to handle this

Yeah, that's the same thing I noticed; that's why I assumed it was "somehow" expected as the undici's one is a variant of the one's in core, and it breaks the contract (I'm assuming here, the one from Node.js core is loaded first on boot, before running the actual script, and that's why they difference).

FWIW v22 and v23 uses Undici v6. Shouldn't we aim 23 (and further 24) become v7?

[mcollina]

Should we just revert the API changes and find a less breaking variation?

I don't think we should revert. My take is that if getGlobalDispatcher() is called and there is a v6 dispatcher there, it gets autowrapped.

I'm assuming here, the one from Node.js core is loaded first on boot, before running the actual script, and that's why they difference

This should not happen, it should be lazily loaded.