adapter-node duplicates code, resulting in missing 413 errors

Rich-Harris commented 8 months ago

Describe the bug

When building a project with adapter-node, we end up with duplicated code. That's because we take the Vite output (which bundles @sveltejs/kit code) and point the built adapter-node files (which also bundles SvelteKit code, via @sveltejs/kit/node) at it.

This is mostly harmless, but it results in a bug when a request body exceeds BODY_SIZE_LIMIT — because of a failed instanceof SvelteKitError check (the error is an instance of the SvelteKitError class defined in the adapter-node bundle, but the instanceof check happens inside the Vite bundle), the wrong status/message are passed to handleError.

There are three possible angles of attack that I can see:

build stuff in such a way that duplication is avoided. This is the correct solution, but I'm not immediately sure how we'd make that work. we could conceivably exclude @sveltejs/kit from the Vite bundle, but then adapters would in many cases need to be responsible for bundling it themselves
allow adapters to influence the Vite config somehow (i.e. specify an entry point that's defined inside the adapter, with a special module ID that points to the default server entry point)
the cheap and nasty solution which I hope we avoid — duck typing SvelteKitError. This would just mask the problem instead of solving it

Reproduction

https://github.com/Rich-Harris/adapter-node-repro

Logs

No response

System Info

System:
    OS: macOS 14.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 61.66 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.2 - ~/Library/pnpm/node
    npm: 9.8.1 - ~/Library/pnpm/npm
    pnpm: 8.10.2 - ~/Library/pnpm/pnpm
    bun: 1.0.10 - ~/.bun/bin/bun
  Browsers:
    Chrome: 120.0.6099.199
    Chrome Canary: 122.0.6236.2
    Safari: 17.2
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.1.0 
    @sveltejs/adapter-node: ^2.1.1 => 2.1.1 
    @sveltejs/kit: ^2.0.0 => 2.3.0 
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.0.1 
    svelte: ^4.2.7 => 4.2.8 
    vite: ^5.0.3 => 5.0.11

Severity

annoyance

Additional Information

No response

Conduitry commented 8 months ago

Would shipping adapter-node unbundled help with this? (We had previously tried doing that so that we could update the undici version people were using simply by updating the dependency in Kit, rather than needing to republish the adapter to bundle the new version into its shims code - but that's now unneeded since we can just rely on the version of undici that comes with Node.) However, we had to revert that because of a bug in Rollup - https://github.com/rollup/rollup/issues/4708. I'm not sure what our other options are.

Rich-Harris commented 8 months ago

It's not enough by itself, as Kit is bundled into the Vite output as well. But it would be a necessary first step I think, yeah

sveltejs / kit