cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.41k stars 590 forks source link

🐛 BUG: `mockOAuthFlow` breaks Miniflare in tests #6132

Open geelen opened 1 week ago

geelen commented 1 week ago

Which Cloudflare product(s) does this pertain to?

Wrangler core, Miniflare

What version(s) of the tool(s) are you using?

3.60.3

What version of Node are you using?

20.10.0

What operating system and version are you using?

Mac 14.5

Describe the Bug

In testing https://github.com/cloudflare/workers-sdk/pull/6073, I had test failures for this production code:

const mf = new Miniflare({
    modules: true,
    script: "export default {}",
    d1Persist,
    d1Databases: { DATABASE: id },
});

The error was:

 FAIL  src/__tests__/d1/export.test.ts > execute > should handle local
TypeError: server.on is not a function
 ❯ module.exports ../../node_modules/.pnpm/stoppable@1.1.0/node_modules/stoppable/lib/stoppable.js:14:12
 ❯ ../miniflare/src/runtime/config/workerd.capnp.js:8716:51
 ❯ Miniflare2.#startLoopbackServer ../miniflare/src/runtime/config/workerd.capnp.js:8715:12
 ❯ Miniflare2.#getLoopbackPort ../miniflare/src/runtime/config/workerd.capnp.js:8708:59
 ❯ Miniflare2.#assembleAndUpdateConfig ../miniflare/src/runtime/config/workerd.capnp.js:8934:53
 ❯ ../miniflare/src/runtime/config/workerd.capnp.js:8555:87
 ❯ Mutex.runWith ../miniflare/src/runtime/config/workerd.capnp.js:3498:25
 ❯ new Miniflare2 ../miniflare/src/runtime/config/workerd.capnp.js:8555:44
 ❯ exportLocal src/d1/export.ts:131:13
    129|  );
    130|
    131|  const mf = new Miniflare({

I jumped into Miniflare to figure out which line was breaking, it was this invocation:

#startLoopbackServer(hostname: string): Promise<StoppableServer> {
    if (hostname === "*") hostname = "::";

    return new Promise((resolve) => {
        const server = stoppable(
            http.createServer(this.#handleLoopback),
            /* grace */ 0
        );
        server.on("upgrade", this.#handleLoopbackUpgrade);
        server.listen(0, hostname, () => resolve(server));
    });
}

Turns out, http.createServer was returning a mock object which had no method .on (which was required by stoppable, otherwise it would have failed a few lines later). That was a side effect of my test calling:

import { mockOAuthFlow } from "../helpers/mock-oauth-flow";

// ...

const { mockOAuthServerCallback } = mockOAuthFlow();

Which, it turns out, I didn't need. So I deleted that and my tests were fine. But adding a mock for oAuth shouldn't break Miniflare, should it? So maybe there's a better way to do that?

Please provide a link to a minimal reproduction

No response

Please provide any relevant error logs

No response