cloudflare / workers-sdk

โ›…๏ธ Home to Wrangler, the CLI for Cloudflare Workersยฎ
https://developers.cloudflare.com/workers/
Apache License 2.0
2.72k stars 712 forks source link

๐Ÿ› BUG: 3.15.0 breaks Remix HMR with "TypeError: error loading dynamically imported module" #4318

Closed KrisBraun closed 1 month ago

KrisBraun commented 1 year ago

Which Cloudflare product(s) does this pertain to?

Workers Runtime, Wrangler core

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

3.15.0

What version of Node are you using?

v18.18.2

What operating system are you using?

Mac

Describe the Bug

3.15.0 has introduced an issue that breaks Remix hot reloading with the browser error "TypeError: error loading dynamically imported module".

While 3.14.0 had a different issue which was fixed in 3.15.0, @izznatsir patched 3.14.0 with the fix, and it doesn't have the "TypeError: error loading dynamically imported module" issue. So it seems another change in 3.15.0 is causing this.

This makes wrangler unusable for Remix dev, so many of us have pegged the wrangler version at 3.10.1 until it's fixed.

The repro repo contains a stock npm create cloudflare@latest my-remix-app -- --framework=remix with wrangler updated. Simple npm run dev and edit a file to trigger the issue.

Please provide a link to a minimal reproduction

https://github.com/KrisBraun/wrangler-3.15.0-remix-issue

Please provide any relevant error logs

No response

mrbbot commented 1 year ago

Hey! ๐Ÿ‘‹ Thanks again for raising this issue. The problematic change in this case appears to be bumping to miniflare@3.20231025.0 and workerd@1.20231025.0 (be0c62834af0692785785cec8a0d7bc9dcfaa61a). workerd@1.20231025.0 includes https://github.com/cloudflare/workerd/pull/1294, which makes workerd's logging more like Node's. In particular, this change means that in local mode, Wrangler no longer uses the V8 inspector for logging, and instead relies on workerd logging to stdout directly. This means that logs in the top level (such as Remix's HMR ready message) will now be output as workerd is starting and evaluating your worker code, not once workerd is ready to accept connections and an inspector is connected.

Essentially, the ready message is logged before workerd is ready to receive connections, and so loading new modules fails because the HTTP server isn't actually listening yet.

Before (working)

[mf:inf] Updated and ready on http://0.0.0.0:8788
[REMIX DEV] 40a9d15f ready

After (broken)

[REMIX DEV] a32ee945 ready
[mf:inf] Updated and ready on http://0.0.0.0:8788

We could fix this by delaying logs until the runtime is ready (https://github.com/cloudflare/workers-sdk/commit/12011ab6f1e73881d06b995c5718d7fcd1b8cf14). This isn't a very nice solution, however, we're currently working on a re-architecture of wrangler dev that will buffer all requests to the local workerd server until it's ready to receive connections using a proxy server (https://github.com/cloudflare/workers-sdk/pull/4413). This should also solve the problem as the browser will make a request to the proxy server, which will wait until workerd is ready before responding with module contents.

For now, I'd recommend you stay on your pinned wrangler version until this is fixed.

/cc @RamIdeas for another example of how the proxy server will be useful in local mode ๐Ÿ™‚

pmbanugo commented 1 year ago

Hi @mrbbot which version of wrangler would you recommend sticking to? 3.14? or 3.9? I moved from 3.9 to 3.15 and currently experiencing this

izznat commented 1 year ago

@pmbanugo 3.10.1

mrbbot commented 11 months ago

we're currently working on a re-architecture of wrangler dev that will buffer all requests to the local workerd server until it's ready to receive connections using a proxy server

Hey everyone! ๐Ÿ‘‹ This change has now landed in wrangler@beta. I've just tested this out and it seems to fix the issue. ๐ŸŽ‰ Would you be able to npm install wrangler@beta and verify this fixes things for you too? Please let us know if you encounter any issues. ๐Ÿ‘

izznat commented 11 months ago

Confirmed to be fixed using the beta version! Thanks!

pmbanugo commented 11 months ago

@mrbbot I can confirm it works for me as well. Thanks! Any idea when this will get to stable release?

mrbbot commented 11 months ago

@pmbanugo Assuming everything goes smoothly, we're hoping to release this early next week ๐Ÿ‘

pmbanugo commented 11 months ago

@pmbanugo Assuming everything goes smoothly, we're hoping to release this early next week ๐Ÿ‘

@mrbbot was this included in v3.18.0? I didn't see any info about that in the release note

mrbbot commented 11 months ago

Hey! ๐Ÿ‘‹ Due to a security hotfix, wrangler 3.18.0 went out last week without these changes. These changes will be released in 3.19.0 which will hopefully be released today/tomorrow. ๐Ÿ‘

pmbanugo commented 11 months ago

Thank you โค๏ธ

supachaidev commented 11 months ago

@mrbbot Please take a look at this https://github.com/remix-run/remix/pull/8288#issuecomment-1857811666, there is still an issue with Remix Cloudflare Workers template. Thank you.

yuriy-yarosh commented 8 months ago

I'm getting similar error with a proxy yarn wrangler pages dev --port 3000 --proxy 5173 -- yarn remix vite:dev

requires setting vite hmr clientPort to 3000 as well:

export default defineConfig(({ mode }) => ({
  plugins: [
    remixCloudflareDevProxy(),
    remix({
      presets: []
    }),
    tsconfigPaths()
  ],
  server: {
    hmr: {
      protocol: 'ws',
      clientPort: 3000
    }
  }
}));

I've actually had been able to narrow it down a bit with --log-level debug

workerd/io/worker-entrypoint.c++:340: info: exception = kj/async-io-unix.c++:186: disconnected: ::write(fd, buffer, size): Broken pipe
workerd/io/worker-entrypoint.c++:340: info: exception = kj/async-io-unix.c++:186: disconnected: worker_do_not_log; Request failed due to internal error
workerd/io/worker-entrypoint.c++:340: info: exception = kj/async-io-unix.c++:186: disconnected: remote.worker_do_not_log; Request failed due to internal error
workerd/server/server.c++:3088: error: Uncaught exception: kj/async-io-unix.c++:186: disconnected: remote.worker_do_not_log; Request failed due to internal error
๎‚ฐ yarn wrangler --version
 โ›…๏ธ wrangler 3.34.2
emily-shen commented 1 month ago

Closing as this seems to be fixed in later versions of wrangler (at least, running the repro with a new version of wrangler seems fine). Please let us know if anything like this pops up again :))