Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adapter-node duplicates code, resulting in missing 413 errors #11590

Open
Rich-Harris opened this issue Jan 10, 2024 · 2 comments
Open

adapter-node duplicates code, resulting in missing 413 errors #11590

Rich-Harris opened this issue Jan 10, 2024 · 2 comments
Labels
bug Something isn't working pkg:adapter-node

Comments

@Rich-Harris
Copy link
Member

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

@Rich-Harris Rich-Harris added bug Something isn't working pkg:adapter-node labels Jan 10, 2024
@Conduitry
Copy link
Member

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 - rollup/rollup#4708. I'm not sure what our other options are.

@Rich-Harris
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:adapter-node
Projects
None yet
Development

No branches or pull requests

2 participants