aws / aws-lambda-nodejs-runtime-interface-client

Apache License 2.0
188 stars 55 forks source link

HttpResponseStream doesn't work when `end`ing the stream without a `write` #97

Open heypiotr opened 9 months ago

heypiotr commented 9 months ago

When using HttpResponseStream to set the status code and headers on a streaming response, I noticed that if I never call write on the stream, the custom status code and headers don't work.

Example repro:

export const handler = awslambda.streamifyResponse(
    async (event, responseStream, context) => {
        const metadata = {
            statusCode: 404,
            headers: { "Content-Type": "text/plain", "X-Foo": "Bar" }
        };
        responseStream = awslambda.HttpResponseStream.from(responseStream, metadata);
        // This will cause a 502 with no custom response headers:
        responseStream.end("Not Found");
        // This will cause a 200 with no custom response headers:
        responseStream.end();
    }
);

I believe this is because HttpResponseStream relies on the onBeforeFirstWrite callback:

https://github.com/aws/aws-lambda-nodejs-runtime-interface-client/blob/7374a4e338fc3b070811d55ee337740f3f6cb382/src/HttpResponseStream.js#L22-L27

onBeforeFirstWrite is implemented by overriding http.ClientRequest stream's write:

// https://github.com/aws/aws-lambda-base-images/tree/nodejs18.x -> /var/runtime/index.mjs
req.write = function(chunk, encoding, callback) {
  vvverbose("ResponseStream::write", chunk.length, "callback:", typeof callback);
  if (typeof chunk !== "string" && !Buffer.isBuffer(chunk) && chunk?.constructor !== Uint8Array) {
    chunk = JSON.stringify(chunk);
  }
  if (status === STATUS_READY && typeof this._onBeforeFirstWrite === "function") {
    this._onBeforeFirstWrite((ch) => origWrite(ch));
  }
  const ret = origWrite(chunk, encoding, callback);
  // [snip]

But turns out Node's ClientRequest doesn't call write when ending the stream with a final chunk of data, it calls an internal write_ instead:

https://github.com/nodejs/node/blob/544cfc5ef151bca8d625fbccc581200a77b00bc0/lib/_http_outgoing.js#L1106

I guess this could also be considered a Node bug, because their documentation for ClientRequest.end says:

If data is specified, it is equivalent to calling request.write(data, encoding) followed by request.end(callback).

But even if it did implement that contract correctly, there's still the case of ending the stream with no data, i.e., responseStream.end().

paya-cz commented 7 months ago

Related to #95