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

Streamified response with empty body ignores other response attributes #1189

Closed
mdenkinger opened this issue Mar 14, 2024 · 3 comments
Closed
Labels

Comments

@mdenkinger
Copy link
Contributor

mdenkinger commented Mar 14, 2024

Describe the bug

When a Lambda function in invoke mode RESPONSE_STREAM and streamifyResponse=true returns a response with an empty body (null or ''), then the other response attributes like statusCode or headers are ignored.

To Reproduce

Sample code:

import { createRequire } from 'module';const require = createRequire(import.meta.url);
import middy from "@middy/core";
import { Readable } from "stream";

function healthcheck() {
  return () => {
    return handler();
  };
}
async function handler() {
  return {
    statusCode: 301,
    body: '', 
    headers: {
      location: 'https://example.com/'
    }
  };
}

var middyHandler = middy({
  streamifyResponse: true
})
.handler(healthcheck());
export {
  middyHandler as handler
};

Empty body cause the Lambda to respond with 200 instead if 301

{
    statusCode: 301,
    body: '', 
    headers: {
      location: 'https://example.com/'
    }
  }

Response headers from client:

>
< HTTP/1.1 200 OK
< Date: Thu, 14 Mar 2024 17:23:24 GMT
< Content-Type: application/octet-stream
< Transfer-Encoding: chunked
< Connection: keep-alive
< x-amzn-RequestId: xxxxx
< X-Amzn-Trace-Id: root=xxx
<

e.g. blank body (one space) causes the Lambda to respond correct with 301 and also the headers are present:

{
    statusCode: 301,
    body: ' ', 
    headers: {
      location: 'https://example.com/'
    }
  }

Response headers from

>
< HTTP/1.1 301 Moved Permanently
< Date: Thu, 14 Mar 2024 17:23:42 GMT
< Content-Type: application/json
< Transfer-Encoding: chunked
< Connection: keep-alive
< x-amzn-RequestId: xxxx
< location: https://example.com/
< X-Amzn-Trace-Id: root=xxxx
<

Expected behaviour

The other response attributes like statusCode, headers,cookies are return with an empty body.

Environment (please complete the following information):

  • Node.js: 20
  • Middy: 5.2.0
  • AWS SDK 3.533.0

Additional context

@mdenkinger mdenkinger added the bug label Mar 14, 2024
@willfarrell
Copy link
Member

Directly to Function URL or through API Gateway?

Empty body cause the Lambda to respond with 200 instead if 301

This is an AWS issue, Middy doesn't override statusCodes within core. Please open a support ticket with AWS, and point them here.

If AWS signals that they won't fix, we could change https://github.com/middyjs/middy/blob/main/packages/core/index.js#L58 to default to as a workaround. I'll have to think on If it should always or only when statusCode is 204, 300 - 399, or others.

PS I recall 3xx statusCodes used to return 500s when it was first released. So, they may be willing to fix.

@mdenkinger
Copy link
Contributor Author

@willfarrell Thanks for checking. It’s through a Function URL. I’ll raise a case a AWS and then update this issue with the response.

@mdenkinger
Copy link
Contributor Author

mdenkinger commented Mar 15, 2024

While working on the AWS case, I dug into the middy code more closely. I think I found a potential problem in how middy handles strings with no content. Basically, when there's no content in the iterator generator does not yield and then the pipeline then does not process the HttpResponseStream. As far as I can see.

To fix this, we can make a simple change. By ensuring the generator always yields content (even if it's just once with no content), we can resolve the issue.

use while (position <= length) instead of while (position < length).

Here's a sample code without middy, but with the algorithm to generate a string response in a stream-like manner.

import util from 'util';
import stream from 'stream';

const {Readable} = stream;
const pipeline = util.promisify(stream.pipeline);


export const handler = awslambda.streamifyResponse(async (event, responseStream, _context) => {

    responseStream = awslambda.HttpResponseStream.from(responseStream, {
        statusCode: 301, headers: {
            location: 'https://example.com'
        }
    });

    let handlerBody = null;
    handlerBody = handlerBody ?? ''

    // Source @datastream/core (MIT)
    let handlerStream
    if (handlerBody._readableState) {
        handlerStream = handlerBody
        console.log('its a stream')
    } else if (typeof handlerBody === 'string') {
        function* iterator(input) {
            const size = 16384 // 16 * 1024 // Node.js default
            let position = 0
            const length = input.length

            while (position <= length) {
                yield input.substring(position, position + size)
                position += size
            }
        }
        console.log('its a string')
        handlerStream = Readable.from(iterator(handlerBody))
    }

    if (!handlerStream) {
        throw new Error('handler response not a ReadableStream')
    }
    await pipeline(handlerStream, responseStream);
});

I had some trouble setting up ava tests on my computer, so I couldn't create a pull request yet. 😞

willfarrell added a commit that referenced this issue Mar 15, 2024
fix: #1189 write prelude in streamifyResponse with string body
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants