next.js
1a07ce20 - fix: bodySizeLimit error responses + limit for non-multipart actions (#77746)

Commit
270 days ago
fix: bodySizeLimit error responses + limit for non-multipart actions (#77746) This PR fixes some bugs in our handling of `serverActions.bodySizeLimit`. There's a couple fixes here, which can be viewed commit by commit. ### 1. Uncaught exception error when exceeding the size limit We were using `body.pipe(busboy)`, which does not forward errors from the source to the sink, so the busboy stream would just hang, and we'd log an error, but never produce a response. It seems like node is (usually) smart enough to abort the request when this happens (because it was triggering an uncaught exception), but we should be returning a proper response instead. This is fixed by using `require('node:stream').pipeline(body, busboy)`, which propagates errors correctly. ### 2. Apply size limit to non-multipart fetch actions We have a separate codepath for non-multipart fetch actions. This can happen if the action arguments are simple enough that react doesn't need to use multiple rows to serialize them -- generally, this means that they're JSON-esque without any complex types like Promises, Maps, or Sets. This codepath was consuming the original request body, not the one piped through the size limit transform. So we'd print the error (because the transform is subscribed to the body), but still execute the action. ### 3. Tests The likely reason that these bugs slipped through is that the tests were only checking if an error was printed to the console, and not checking if the server actually responded with an error. To prevent this, I've added some assertions on the responses' status codes + checking whether an error boundary was triggered. I've also tweaked the tests so that the parts that submit actions can run in deploy mode. The request interception code proved subtle/complicated enough that i decided to pull it out into a separate helper. We have a whole bunch of tests that intercept requests/responses using various ad-hoc tangles of `page/browser.on('request', ...)` which i'd like to migrate to this helper, because that'd make them easier to understand, but I'll leave that for a follow up when there's time.
Author
Parents
Loading