Node: Processing a request response with HTTP 204 fails with HPE_INVALID_CONSTANT if the response has content

[filipecatraia]

• Version: v10.13.0 (also with v8.9.0)
• Platform: Darwin MacBook-Pro.local 18.2.0 Darwin Kernel Version 18.2.0: Fri Oct 5 19:41:49 PDT 2018; root:xnu-4903.221.2~2/RELEASE_X86_64 x86_64

Error: Parse Error
    at Socket.socketOnData (_http_client.js:441:20)
    at Socket.emit (events.js:182:13)
    at addChunk (_stream_readable.js:283:12)
    at readableAddChunk (_stream_readable.js:264:11)
    at Socket.Readable.push (_stream_readable.js:219:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:94:17)
    ...
    code: 'HPE_INVALID_CONSTANT',
    ...

I found similar reports on multiple libraries. In fact, this could just be a duplicate of these:

• https://github.com/nodejs/node-v0.x-archive/issues/4863
• https://github.com/nodejs/node-v0.x-archive/issues/8514

Sorry if that's the case.

We have a PHP API backend using Symphony, and the endpoints use Symfony\Component\HttpFoundation\JsonResponse. When a request is accepted, we return HTTP 204 No Content, via new JsonResponse() along with JsonResponse::HTTP_NO_CONTENT. This, however, returns a content length of 2: {}. When that's the case, node fails parsing the response. In contrast, _curl_ and _Postman/Insomnia_ have no issue with that. They simply ignore the content and close the connection.

The issue here is that 204 means a request was accepted, but node throws an error and the promise waiting for the response fails. After some investigation, I decided to use Symfony\Component\HttpFoundation\Response, and return the response as Response::HTTP_NO_CONTENT. The response now has a proper content length of 0, so node can process the response adequately.

I'm pasting the full stack below, sorry if that's excessive.

{ Error: Parse Error
    at Socket.socketOnData (_http_client.js:441:20)
    at Socket.emit (events.js:182:13)
    at addChunk (_stream_readable.js:283:12)
    at readableAddChunk (_stream_readable.js:264:11)
    at Socket.Readable.push (_stream_readable.js:219:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:94:17)
  bytesParsed: 308,
  code: 'HPE_INVALID_CONSTANT',
  config:
   { adapter: [Function: httpAdapter],
     transformRequest: { '0': [Function: transformRequest] },
     transformResponse: { '0': [Function: transformResponse] },
     timeout: 0,
     xsrfCookieName: 'XSRF-TOKEN',
     xsrfHeaderName: 'X-XSRF-TOKEN',
     maxContentLength: -1,
     validateStatus: [Function: validateStatus],
     headers:
      { Accept: 'application/json, text/plain, */*',
        'Content-Type': 'application/json',
        'User-Agent': 'axios/0.18.0',
        'Content-Length': 117 },
     method: 'post',
     url: 'http://localhost:8083/config/wip.booking_funnel.enable_m2',
     data:
      '{"value":false,"description":"WAA-491 - Enable Booking funnel Milestone 2 functionalities","example":"true or false"}' },
  request:
   Writable {
     _writableState:
      WritableState {
        objectMode: false,
        highWaterMark: 16384,
        finalCalled: false,
        needDrain: false,
        ending: false,
        ended: false,
        finished: false,
        destroyed: false,
        decodeStrings: true,
        defaultEncoding: 'utf8',
        length: 0,
        writing: false,
        corked: 0,
        sync: true,
        bufferProcessing: false,
        onwrite: [Function: bound onwrite],
        writecb: null,
        writelen: 0,
        bufferedRequest: null,
        lastBufferedRequest: null,
        pendingcb: 0,
        prefinished: false,
        errorEmitted: false,
        emitClose: true,
        bufferedRequestCount: 0,
        corkedRequestsFree: [Object] },
     writable: true,
     _events:
      { response: [Function: handleResponse],
        error: [Function: handleRequestError] },
     _eventsCount: 2,
     _maxListeners: undefined,
     _options:
      { protocol: 'http:',
        maxRedirects: 21,
        maxBodyLength: 10485760,
        path: '/config/wip.booking_funnel.enable_m2',
        method: 'post',
        headers: [Object],
        agent: undefined,
        auth: undefined,
        hostname: 'localhost',
        port: '8083',
        nativeProtocols: [Object],
        pathname: '/config/wip.booking_funnel.enable_m2' },
     _redirectCount: 0,
     _redirects: [],
     _requestBodyLength: 117,
     _requestBodyBuffers: [],
     _onNativeResponse: [Function],
     _currentRequest:
      ClientRequest {
        _events: [Object],
        _eventsCount: 6,
        _maxListeners: undefined,
        output: [],
        outputEncodings: [],
        outputCallbacks: [],
        outputSize: 0,
        writable: true,
        _last: true,
        chunkedEncoding: false,
        shouldKeepAlive: false,
        useChunkedEncodingByDefault: true,
        sendDate: false,
        _removedConnection: false,
        _removedContLen: false,
        _removedTE: false,
        _contentLength: null,
        _hasBody: true,
        _trailer: '',
        finished: true,
        _headerSent: true,
        socket: [Socket],
        connection: [Socket],
        _header:
         'POST /config/wip.booking_funnel.enable_m2 HTTP/1.1\r\nAccept: application/json, text/plain, */*\r\nContent-Type: application/json\r\nUser-Agent: axios/0.18.0\r\nContent-Length: 117\r\nHost: localhost:8083\r\nConnection: close\r\n\r\n',
        _onPendingData: [Function: noopPendingOutput],
        agent: [Agent],
        socketPath: undefined,
        timeout: undefined,
        method: 'POST',
        path: '/config/wip.booking_funnel.enable_m2',
        _ended: true,
        res: [IncomingMessage],
        aborted: undefined,
        timeoutCb: null,
        upgradeOrConnect: false,
        parser: null,
        maxHeadersCount: null,
        _redirectable: [Circular],
        [Symbol(isCorked)]: false,
        [Symbol(outHeadersKey)]: [Object] },
     _currentUrl: 'http://localhost:8083/config/wip.booking_funnel.enable_m2' },
  response: undefined }

[Trott]

@nodejs/http

[mscdex]

The latest HTTP/1.1 RFC says:

A 204 response is terminated by the first empty line after the header fields because it cannot contain a message body.

So I think node is doing the right thing here.

[filipecatraia]

@mscdex You're correct, but the response isn't just being terminated. That would be fine. The issue is that because an error is thrown, any promise with the request fails (goes to catch). In reality, the request succeeded. This means an HTTP 204 with content is treated as an exception.

[mscdex]

If you're handling errors, shouldn't you be paying attention to the error instead of making assumptions about what the error is in your catch() handler?

[Trott]

This means an HTTP 204 with content is treated as an exception.

I may be misunderstanding something here, so apologies in advance if that's the case, but my gut reaction to this is: While that's not what curl etc. does, it seems like correct behavior. I think I'd prefer Node.js report that the response incorrectly contained a message body and allow the end user to determine if they want to ignore it silently or not, then force everyone to silently ignore the presence of a message body in a response that is not supposed to have one.

Am I misunderstanding something or failing to take something into consideration?

[Trott]

(Although I guess if that's the case, a less cryptic error message would probably be useful.)

[filipecatraia]

@Trott Yes, I still agree with you on principle 🙂 I'm not sure whether an HTTP 204 with content should be treated as an exception or not. I just think it's a good idea to report the behaviour, since the parser is completely choking on the request.

Ideally, the 204 is still a valid response that would result in a promise resolving, even if it contains content. But again, I'm not sure. You're more experienced with the spec, but maybe yeah, returning a better error message would be ok. Even if a promise would be rejected and land on a catch statement, the error object could contain a error.response or error.status to check if the status is what we expected.

[devsnek]

the usage of terminate in the http spec is slightly ambiguous, but i think in this case it means when the parser should stop interpreting the response buffer, not that the entire http request/response should be killed.

sidenote: you should also probably file a bug with the php library about it returning a body with 204

[mscdex]

@devsnek I think the key phrase in the spec is (emphasis mine):

because it cannot contain a message body

[filipecatraia]

@mscdex Again, I generally agree, but the entire request gets rejected. As @devsnek pointed out, it would be saner and more graceful to stop processing the request, not throw. Or, throw, but in a way that allows the developer to work out what happened. As it is, it took me roughly 2 hours to figure out what was going on. And there are similar reports in libraries built on top of Node. But again, I defer to the more experienced folk :)

[mscdex]

After digging in Symfony's code a bit it looks like what's actually happening is that it's removing Content-Type and Content-Length headers when the response code is 204, but sending the data anyway. So this is definitely a bug in Symfony.

However in my testing I still get the 'response' event before the 'error' event on the client side, so you should be able to work around this however you see fit FWIW.

[filipecatraia]

@mscdex Nice, thanks for the input! I'll report the bug in Symfony, though a simple workaround is to use Response instead of JsonResponse for HTTP 204 codes.

[apapirovski]

This is defined as "MUST NOT" in the http spec. While the behavior is undefined, I don't believe that means we should treat 204s with a body as valid and just ignore that body.

I'm going to close this out since this doesn't seem actionable, especially in light of @mscdex pointing out that a response event is still emitted.