fix: handle 304 responses from functions #373
Conversation
| }, | ||
| test: { | ||
| include: ['src/**/*.test.ts'], | ||
| include: ['(src|dev)/**/*.test.ts'], |
There was a problem hiding this comment.
packages/functions/dev/main.test.ts was not running, with this added to config, other than added failing test (expected), it seems like some other tests in this module are failing/timing out :/ https://github.com/netlify/primitives/actions/runs/16414390123/job/46376826143?pr=373#step:11:301
| settings: {}, | ||
| timeouts: {}, | ||
| userFunctionsPath: 'netlify/functions', | ||
| watch: true, |
There was a problem hiding this comment.
Test was timing out here
primitives/packages/functions/dev/main.test.ts
Lines 43 to 44 in 4386ac8
Same situation for similar addition in 'Returns a `preferStatic` property' test
| // For streaming responses, lambda-local always return a result with body stream. | ||
| // We need to discard this if response status code does not allow to have a body. | ||
| const shouldHaveBodyStream = !nullBodyStatus.includes(invocationResult.statusCode) | ||
|
|
||
| if (shouldHaveBodyStream) { |
There was a problem hiding this comment.
Note, that we have no such handling in CLI, but difference is that we convert to Response here, while CLI is using http.ServerResponse. In http.ServerResponse case it seems like body is just silently discarded, while Response will actively throw
| import sourceMapSupport from 'source-map-support' | ||
|
|
||
| // https://github.com/nodejs/undici/blob/a36e299d544863c5ade17d4090181be894366024/lib/web/fetch/constants.js#L6 | ||
| const nullBodyStatus = [101, 204, 205, 304] |
There was a problem hiding this comment.
nit: Maybe make this a Set for a nicer lookup?
| // We need to discard this if response status code does not allow to have a body. | ||
| const shouldHaveBodyStream = !nullBodyStatus.includes(invocationResult.statusCode) | ||
|
|
||
| if (shouldHaveBodyStream) { |
There was a problem hiding this comment.
nit: If we could invert this check and early-return, we'd avoid an extra level of nesting.
There was a problem hiding this comment.
I pushed a change. Had to do slightly larger change, because there were no functions in this worker module to begin with so couldn't really early-return without it, but I think it's slightly nicer now (tho I might have invited opportunity for more NITs doing that :) )
Quite similar problem to #364, just fix being a little less nice having to rely on hardcoded array of bodyless status codes instead of being able to just check if body exists