awslabs / aws-lambda-go-api-proxy

lambda-go-api-proxy makes it easy to port APIs written with Go frameworks such as Gin (https://gin-gonic.github.io/gin/ ) to AWS Lambda and Amazon API Gateway.
Apache License 2.0
1.04k stars 197 forks source link

At fiber adapter, to add header method is changed 'Add(k, v)' to 'Set(k, v)' #89

Closed drakejin closed 2 years ago

drakejin commented 3 years ago

Issue #, if available:

88

Description of changes: Fiber lambda adapter cannot pass specific header 'Content-Type', 'User-Agent'. So, I change method Add to Set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

drakejin commented 3 years ago

@sapessi Would you check my PR? And. How can I pass this CI?

zoltanarvai commented 3 years ago

@sapessi Would you check my PR? And. How can I pass this CI?

you just need to run go mod tidy.

greeeg commented 3 years ago

Hello everyone 👋 First, thanks for your contributions on this project. I wanted to know the state of this PR? This fix would be greatly appreciated by Go Fiber & AWS Lambda users :)

sapessi commented 3 years ago

@drakejin could you run go mod tidy as suggested by @zoltanarvai to see if that fixes the CI issues?

drakejin commented 3 years ago

@sapessi @greeeg Sorry, I'm tooooooo late.

It's little weird. If I execute this command go mod tidy, There are changed lines a lot. Do you want me to run this command and commit?

image

sapessi commented 3 years ago

Yes, go ahead and add that file @drakejin - if this doesn't fix the issue I'll check out the branch and see if I can resolve it manually.

sapessi commented 3 years ago

Looks like the mod tidy fixed the issue we had with the build. Now one of the tests for fiber is failing:

FiberLambda tests
235/home/travis/gopath/src/github.com/awslabs/aws-lambda-go-api-proxy/fiber/fiberlambda_test.go:15
236  Simple ping request
237  /home/travis/gopath/src/github.com/awslabs/aws-lambda-go-api-proxy/fiber/fiberlambda_test.go:16
238    Proxies the event correctly [It]
239    /home/travis/gopath/src/github.com/awslabs/aws-lambda-go-api-proxy/fiber/fiberlambda_test.go:17
240
241    Expected
242        <int>: 404
243    to equal
244        <int>: 200
245
246    /home/travis/gopath/src/github.com/awslabs/aws-lambda-go-api-proxy/fiber/fiberlambda_test.go:60
247------------------------------
greeeg commented 3 years ago

Any update on this @drakejin?

drakejin commented 3 years ago

@greeeg Sorry, I'm late, It's something wired, @kiyonlin ctx.Next() is changed response status code. I wonder why. Would you please check this?

https://github.com/awslabs/aws-lambda-go-api-proxy/pull/89/files#diff-49b5cac158fb0c9ddfb54c5d7255995099ddc088041214c1720d0c6816c7351bR22

ThalysonR commented 2 years ago

Hello Any update on this PR?

drakejin commented 2 years ago

@ThalysonR ... hm.. As you know as I'm not fiber contributer. I recommend to use other...... http adapter... sorry fiber. I was changed long time ago.

response header issue is successfully done at this PR. But, status code cannot changed when It middleware ctx.Next() So, I think fiber is not good for AWS lambda adapter.

ThalysonR commented 2 years ago

Oh, I see Thanks for the response anyway, I'll try another adapter

drakejin commented 2 years ago

I'll close this PR @kiyonlin. Sorry, I'll back-off. Many people who use loved fiber are waiting this issue. I hope to resolve this issues by fiber team. (pass specific response header and status code).

My patient was totally exhausted. I asked many times "how should I do about this issue?" with mentions at Discord. but, nobody answer my question. So, I'll back off about this issue. I'll close for other developers who waiting this issues.

Actually, I respect fiber team. I loved fiber's philosophy. So, I expect fiber v3. I hope it didn't occur like this issue.

cc @ReneWerner87

ReneWerner87 commented 2 years ago

hi, unfortunately had overlooked this PR

Unfortunately there are not many people in the fiber project who are very active and we have a lot of requests for support in the main repository and discord and also pull requests (in addition to the normal work, because this work is free and not paid).

so most of the time i don't have the problems from the side projects of fiber on my radar or the focus changes quickly due to important adjustments to the main software

sorry for that, i am depending on your understanding and support

now to the PR, what else has been necessary here, then i'll work on it on the weekend -> think it just needs to be updated and checked again

@drakejin thanks for the code you produced there, will take it over if you don't have time for the refresh and create a new PR, which will hopefully go live in a timely manner

drakejin commented 2 years ago

@ReneWerner87 Hmmm.... Okay!, I'll make new a PR and work next week. Thanks for your service. :)

ReneWerner87 commented 2 years ago

@ReneWerner87 Hmmm.... Okay!, I'll make new a PR and work next week. Thanks for your service. :)

just ping me if you need me on the new pull request and I don't respond