Chunked Encoding Not Supported

[shellscape]

I ran into the Error: chunked encoding not supported message this evening. The error message is vague enough as to not be useful (and I mean that as sincere constructive criticism without snark) and sent me Googling. Given that the error has a test in this package, a FAQ here that included that error message would have gone a long way. I'd recommend the same for any other errors you test for in the future - that makes for good DX. For anyone who happens across this issue, the solution is to delete the 'transfer-encoding' header before sending a response.

While I haven't been able to dig up exactly why chunked encoding isn't supported, it's relatively well-known in various issues that chunked encoding is not supported by API Gateway. However, Amazon has improved the developer experience in aws-serverless-express, and I'd like to suggest you all follow their lead in this package. The code from their repo which handles this case is at:

https://github.com/awslabs/aws-serverless-express/blob/ecdafec8f05eecc97791930b26d93e63a84e6188/src/index.js#L80-L82

      if (headers['transfer-encoding'] === 'chunked') {
        delete headers['transfer-encoding']
      }

One might think it dicey to modify headers, but it went through some discussion on Amazon's repo before they pulled the trigger on the change.

[dougmoscrop]

Yeah agreed about the error message.

I'm not sure we can mimic - transfer encoding is an intermediary header and since aws serverless express runs an actual HTTP server in the Lambda container it is in effect another "node" but this library works in-memory so to speak. I'm up to making it an option that can be enabled but defaults to a safe value. My concern is whatever is setting that header is going to assume it is supported, and with so many different frameworks I think some of them inevitably write directly to the socket - so your body could be corrupted (it would have extra bytes interspersed throughout it denoting the chunk lengths). There's sort of a philosophical crossroads of how these libraries approach the situation.

Either way improving the error message is the right thing to do.

We could also parse/iterate over the chunked encoding before stringifying it, omitting the chunking.

[alexbeletsky]

I experience the same issue and will be happy for advice.. Would it be enough just to suppress 'transfer-encoding' header? @shellscape

[shellscape]

@alexbeletsky here's what we're doing:

app.use(async (ctx: KoaContext, next: KoaNext) => {
  ctx.set('transfer-encoding', '');
  await next();
});

[dougmoscrop]

yeah, that seems better than manipulating the response* header, the important part is that if the application side thinks it can use chunked, you will get corrupt data (if you just hide the header)

[aaronhuisinga]

Looking forward to see what happens here - we're running into the same issue and are also just removing the header for the time being.

[fooman]

Just to confirm that just deleting the headers

      if (headers['transfer-encoding'] === 'chunked') {
        delete headers['transfer-encoding']
      }

by itself is not a full solution. I ended up with what I believe is the chunk sizes being shown in the middle of a json response. So something like this

55b
{
   "expand" : "attributes",
   "link" : {
      "rel" : "self",
      "href" : "http://localhost:8095/crowd/rest/usermanagement/1/user?username=my_username"
   },
   "name" : "my_username",
   "first-name" : "My",
   "last-name" : "Username",
   "display-name" : "My Username",
   "email" : "user@example.test",
55b
   "password" : {
      "link" : {
         "rel" : "edit",
         "href" : "http://localhost:8095/crowd/rest/usermanagement/1/user/password?username=my_username"
      }
   },
   "active" : true,
   "attributes" : {
74
      "link" : {
         "rel" : "self",
         "href" : "http://localhost:8095/crowd/rest/usermanagement/1/user/attribute?username=my_username"
      },
      "attributes" : []
   }
}
0

[davidkarlsen]

I think the least useful is to raise an exception - let api-gateway do that instead. But the docs at https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-known-issues.html seem to hint that they will drop the header from any responses - that doesn't work out since an error is raised instead. Could it be an option to do so at least?

[dougmoscrop]

What do you expect to happen when you do this?

[davidkarlsen]

I expect api-gw to either: a) drop the transfer-encoding from the response (the documented behaviour) or b) raise an error

[davidkarlsen]

Should I create a PR where this can be toggled?

[dougmoscrop]

The problem is that allowing your application to think that it can chunk will lead you to the same error as the comment above you.. the body will be corrupted.

[dougmoscrop]

The better way to solve this would, I think, be to dechunkify the body as it's being read.

[davidkarlsen]

I believe this is what aws api-gw does - but never gets to do as the library raises an error instead.

[dougmoscrop]

I feel like it doesn't, but I could either be misremembering or its changed - feel free to remove the error if the result is the same

[davidkarlsen]

I checked it now, using the patch referenced, the response will pass, even if the backend set Transfer-Encoding chunked - and this is in line with AWS' docs.

[shellscape]

@davidkarlsen @dougmoscrop what do we think about that patch at this point?

[fooman]

The better way to solve this would, I think, be to dechunkify the body as it's being read.

Agree with this. If we just remove the chunked encoding header (or let api-gw do it) we end up with a mangled response. I believe there may be a chance that just removing the header works in some circumstances, but that may be related to how many chunks there are. In other words if there is only one it might work.

[shellscape]

Yeah I've never seen a mangled response using that approach.

[davidkarlsen]

Yea. I think the response needs to be handled (read) in the library and the header removed - else it did not work out.

[davidkarlsen]

i. e. The patch is not good to go as is

[shellscape]

@davidkarlsen @dougmoscrop I think we can close this given #207