Empty cookies are set

[jormaechea]

Due to this lines:

serverless-offline/src/api-gateway/ApiGateway.js

Lines 632 to 639 in 858da03

	if (
	typeof headerValue === 'undefined' ||
	headerValue === null
	) {
	debugLog(
	`Warning: empty value for responseParameter "${key}": "${value}"`,
	)
	headerValue = ''

serverless-offline/src/api-gateway/ApiGateway.js

Lines 658 to 659 in 858da03

	debugLog(`Will assign "${headerValue}" to header "${headerName}"`)
	response.header(headerName, headerValue)

If you configure a function like this:

hello:
  handler: index.handler
  events:
    - http:
        path: hello
        method: get
        integration: lambda
        responses:
          default:
            responseModels:
              application/json;charset=UTF-8: Empty
            responseParameters:
              method.response.header.Set-Cookie: "integration.response.body.headers.Set-Cookie"
            responseTemplates:
              application/json;charset=UTF-8: ""
            statusCode: 200

This sets a Cookie with no name and no value. Next request, it throws an error of invalid cookie received. (Header Set-Cookie: )
It's quite annoying, but luckily easy to solve, by simply checking if headerValue is not empty.

If you're OK with it, I can make 2 PRs, for v5.x and v6. (/cc @dnalborczyk )
Let me know what you prefer.!

[dnalborczyk]

hey @jormaechea thank you!

yeah, that would be very much appreciated! a PR against master would be best, if you want the fix for v5 as well, a PR against v5.x-release would be needed. The code regarding any of the lambda-integration hasn't changed (diverted) much yet, but it's really needed. should be fairly easy to navigate around that. thanks a lot!

[jormaechea]

@dnalborczyk Both PRs are ready to be merged! 🚀
(According to Travis at least)

[dnalborczyk]

thanks again @jormaechea !

[jormaechea]

Thank you! I'll be waiting for the npm release 💪

Hey, i've been seeing the lambda integration code and it really needs some refactor..
I could do that if you want.. Actually, I feel like i'm almost the only one using it.. LOL
What do you think?

[dnalborczyk]

@jormaechea

Thank you! I'll be waiting for the npm release 💪

v5.11.0 is out. v6 alpha has to wait a bit as I want to pull in #776

Hey, i've been seeing the lambda integration code and it really needs some refactor..

speaking of which: #776 (comment)

Actually, I feel like i'm almost the only one using it.. LOL

Hard to tell, I would assume that the majority probably uses proxy integration, as it easier to get going, and also recommended by serverless (it's also the integration default). But there are definitely others who it as well. 😄

I could do that if you want.. What do you think?

that would be absolutely amazing! and yes, it's needed VERY BADLY.

could you do it in bits and pieces, so it's easier to follow and to merge? I assume that you agree that it only makes sense in master.

regarding lambda integration:
I was also wondering why VelocityContext and LambdaIntegrationEvent is being used several times, separate, and combined:
https://github.com/dherault/serverless-offline/blob/v6.0.0-alpha.39/src/api-gateway/ApiGateway.js#L508
https://github.com/dherault/serverless-offline/blob/v6.0.0-alpha.39/src/api-gateway/ApiGateway.js#L703
https://github.com/dherault/serverless-offline/blob/v6.0.0-alpha.39/src/api-gateway/lambda-events/LambdaIntegrationEvent.js#L12

I'm thinking that would be a great starting point.

There's quite a bit of files and functionality where I'm not quite sure what they are doing and what they are being used for (mostly lambda integration related). keep in mind the file and folder structure is not set in stone, and can be, or should be re-organized where it makes sense.

let me know if you have any questions!

[dnalborczyk]

@jormaechea

I forgot, I have to commit something for the ES6 module translation to commonjs, for easier development and debugging. I'm using the amazing esm module (https://github.com/standard-things/esm), but it should also work with @babel/register and other tools.

I need to document the development, build, test and release steps as well. I'll get back to you on that soon.

[jormaechea]

Yeah, it would be nice to have some of that documented. I've been taking a look at the __tests__ dir and it's not... How can I say... Intuitive.. 😛

On the other hand, I realized that ruby (and many other things) have to be installed in my local machine so that npm t can run without errors. Have you ever thought about implementing docker for that? I think it would simplify a lot the developing process.

I'll be taking a look at the Lambda integration code, it's flow and get back to you.

keep in mind the file and folder structure is not set in stone

Is Prettier config set in stone? It kinda hurts my eyes.. 😂

[dnalborczyk]

Yeah, it would be nice to have some of that documented. I've been taking a look at the __tests__ dir and it's not... How can I say... Intuitive.. 😛

I agree. keep in mind that part of the refactoring is also to keep everything already existing functioning, including the existing tests. Regarding tests and structure is still plenty to do as well. Another todo list item is to run the tests against AWS itself, so functionality would be base lined on that. (not necessarily automated in a build pipeline, as it requires an AWS account).

On the other hand, I realized that ruby (and many other things) have to be installed in my local machine so that npm t can run without errors.

the python and ruby tests are fairly new, and didn't exist at all. I wanted to have at least some basic tests in place. I'm not a python nor ruby user at all. I have not tried to run the tests on a machine without the frameworks installed. After hearing some issues from other contributors I did put a check in place, and the tests (in theory) should only run when the runtime exists. https://github.com/dherault/serverless-offline/blob/master/__tests__/integration/ruby/ruby.test.js#L9 But like I said, I have not given it a try and don't know if it works or not (yet, but I'll have a look).

Have you ever thought about implementing docker for that? I think it would simplify a lot the developing process.

Not sure what you mean. running python and ruby handlers in a docker container? or running the tests in a docker container?

I'll be taking a look at the Lambda integration code, it's flow and get back to you.

👍 awesome! thanks!

keep in mind the file and folder structure is not set in stone. Is Prettier config set in stone? It kinda hurts my eyes.. 😂

what exactly is hurting your eyes? 😄 there isn't really much of a config - that's the beauty of it. quote:

Prettier is an opinionated code formatter. It enforces a consistent style by parsing your code and re-printing it with its own rules that take the maximum line length into account, wrapping code when necessary.

I'm gonna add some information about the dev setup (es6 modules etc.) as well as the current folder structure and test structure, build setup etc. Also how the handler runners work (in-process, child process, worker thread etc.).

[dnalborczyk]

just checked on an alpine docker container without python and ruby. this function doesn't seem to work: https://github.com/dherault/serverless-offline/blob/master/src/utils/detectExecutable.js

gonna look into it and fix it. btw, you can always get around the git hooks with --no-verify.

e.g.: git push --no-verify

update: detect ... function is async, await is missing :/