Fix local registry being able to start only once in unstable_dev

[kuba-orlik]

When using unstable_dev, it's not possible to start multiple workers in a sequence. Once you stop one worker, the local registry is closed and cannot be started again. This fixed that by clearing the server variable once the server is stopped

[changeset-bot]

🦋 Changeset detected

Latest commit: ad05ab5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
wrangler	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[rozenmd]

Thanks for the PR @kuba-orlik !

Do you mind rebasing onto main or merging main into this branch? I just fixed an issue with a test that'll fail for external contributors.

[kuba-orlik]

Rebase complete :)

[github-actions]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4575668158/npm-package-wrangler-2044

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/2044/npm-package-wrangler-2044

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4575668158/npm-package-wrangler-2044 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4575668158/npm-package-cloudflare-pages-shared-2044

Note that these links will no longer work once the GitHub Actions artifact expires.

[codecov]

Codecov Report

Merging #2044 (ad05ab5) into main (0a77990) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2044      +/-   ##
==========================================
+ Coverage   73.54%   73.59%   +0.04%     
==========================================
  Files         167      167              
  Lines       10499    10500       +1     
  Branches     2807     2807              
==========================================
+ Hits         7722     7727       +5     
+ Misses       2777     2773       -4     

Impacted Files	Coverage Δ
packages/wrangler/src/dev-registry.ts	67.81% <100.00%> (+0.37%)	⬆️

... and 3 files with indirect coverage changes

[rozenmd]

by the way @kuba-orlik - any chance you could share a code sample of what you're trying to do?

[kuba-orlik]

Not necessarily a code sample, but the gist of it is that I'm writing e2e tests for a worker. I want to run it multiple times during the tests, to see how it behaves with various configs.

My tests were failing, because running .stop() on a worker irreversibly killed the local worker registry. Any worker started afterwards kept failing to register and polluting the stderr with error messages related to that fact

[rozenmd]

Ah okay, that makes sense, I forgot folks might want to turn a server on/off throughout a test 😅

I'll try make a test case to reproduce the issue tomorrow

[rozenmd]

Actually @kuba-orlik can you post a minimal reproduction of the issue? I'm not able to see the same error with the following test in Jest:

const { unstable_dev } = require("wrangler");

describe("Worker", () => {
  let worker;

  it("should return Hello World", async () => {
    worker = await unstable_dev(
      "src/index.js",
      {},
      { disableExperimentalWarning: true }
    );
    let resp = await worker.fetch();
    if (resp) {
      const text = await resp.text();
      expect(text).toMatchInlineSnapshot(`"Hello World!"`);
    }
    await worker.stop();

    worker = await unstable_dev(
      "src/index.js",
      {},
      { disableExperimentalWarning: true }
    );
    resp = await worker.fetch();
    if (resp) {
      const text = await resp.text();
      expect(text).toMatchInlineSnapshot(`"Hello World!"`);
    }
    await worker.stop();
  });
});

[kuba-orlik]

This is a kind-of minimal reproduction of the problem:

const { unstable_dev } = require('wrangler')
const config = {
  config: '/tmp/wrangler-test.toml',
  bundle: false,
  logLevel: 'debug',
  forceLocal: true,
  localProtocol: 'http',
  port: 1337,
}

;(async function () {
  let worker = await unstable_dev('dist/worker.js', config, {
    disableExperimentalWarning: true,
  })
  await worker.stop()

  worker = await unstable_dev('dist/worker.js', config, {
    disableExperimentalWarning: true,
  })
  resp = await worker.fetch()
  await worker.stop()
})()

The code itself works, but after creating the second worker onwards it throws errors into stderr, like:

Failed to register worker in local service registry TypeError: fetch failed
    at Object.processResponse (/home/kuba/projects/cloudflare/zaraz-worker/node_modules/wrangler/wrangler-dist/cli.js:8846:27)
    at /home/kuba/projects/cloudflare/zaraz-worker/node_modules/wrangler/wrangler-dist/cli.js:9178:42
    at node:internal/process/task_queues:140:7
    at AsyncResource.runInAsyncScope (node:async_hooks:203:9)
    at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  cause: Error: connect ECONNREFUSED 127.0.0.1:6284
      at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1300:16) {
    errno: -111,
    code: 'ECONNREFUSED',
    syscall: 'connect',
    address: '127.0.0.1',
    port: 6284
  }
}

The errors don't break the flow of the tests, but they do pollute their output

[petebacondarwin]

Is this PR still relevant? I think there was some work on DevRegistry recently to change this behaviour?

[kuba-orlik]

It's still relevant. The error Failed to register worker in local service registry TypeError: fetch failed error message still appears when using 2.8.0

[petebacondarwin]

I have added a changeset and a test. Note that the test file was changed from .js to .ts which is why it looks like a file got deleted and added. The only change is an additional test and a couple of minor tweaks to make the linters happy.

[JacobMGEvans]

I paired with you on this. It is a simple change with important fix.