Fix duplicate port in 'config.GatewayAddress' and misplaced 'return' causing panics.

bmcustodio commented 4 years ago

Description

Currently, gateway_invoke=true causes the app to panic because...

... the gateway port appears twice in the function URL, causing an error while invoking.
... the error handling code seems to have a misplaced return statement, which causes a nil-pointer dereference.

(...)
[#1] Received on [faas-request]: 'sequence:4 subject:"faas-request" data:"{\"Header\":{\"Accept-Encoding\":[\"gzip\"],\"Content-Length\":[\"4\"],\"Content-Type\":[\"text/plain\"],\"User-Agent\":[\"Go-http-client/1.1\"],\"X-Call-Id\":[\"98d0c49d-a33e-48b5-bb7a-75e312338387
\"],\"X-Callback-Url\":[\"http://10.35.109.179/anything\"],\"X-Start-Time\":[\"1574699946469186903\"]},\"Host\":\"127.0.0.1:8080\",\"Body\":\"cXV4Cg==\",\"Method\":\"POST\",\"Path\":\"\",\"QueryString\":\"\",\"Function\":\"figlet\",\"CallbackUrl\":{\"Scheme\":\"http\",\"$
paque\":\"\",\"User\":null,\"Host\":\"10.35.109.179\",\"Path\":\"/anything\",\"RawPath\":\"\",\"ForceQuery\":false,\"RawQuery\":\"\",\"Fragment\":\"\"}}" timestamp:1574699946470133496 redelivered:true '
Invoking: figlet, 4 bytes.
Invoked: figlet [503] in 0.000193s
Post http://gateway.openfaas.svc.cluster.local:8080:8080/function/figlet/: dial tcp: lookup gateway.openfaas.svc.cluster.local:0: no such host
Callback to: http://10.35.109.179/anything
Posted result: 200
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0x6c6c36]

goroutine 25 [running]:
main.main.func1(0xc000050b40)
        /go/src/github.com/openfaas/nats-queue-worker/main.go:153 +0x826

Motivation and Context

[ ] I have raised an issue to propose this change (required)

N/A

How Has This Been Tested?

By building the image (bmcstdio/openfaas-queue-worker:gateway-addr), deploying it and making sure the behaviour is fixed.

(...)
[#1] Received on [faas-request]: 'sequence:18 subject:"faas-request" data:"{\"Header\":{\"Accept-Encoding\":[\"gzip\"],\"Content-Length\":[\"4\"],\"Content-Type\":[\"text/plain\"],\"User-Agent\":[\"Go-http-client/1.1\"],\"X-Call-Id\":[\"267dafe4-d0f9-4308-92d7-04718d594bf
e\"],\"X-Start-Time\":[\"1574702158491557701\"]},\"Host\":\"127.0.0.1:8080\",\"Body\":\"YmF6Cg==\",\"Method\":\"POST\",\"Path\":\"\",\"QueryString\":\"\",\"Function\":\"figlet\",\"CallbackUrl\":null}" timestamp:1574702158492447449 '
Invoking: figlet, 4 bytes.
Wrote 108 Bytes
200 OK
Invoked: figlet [200] in 0.014991s

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I've read the CONTRIBUTION guide
[x] I have signed-off my commits with git commit -s
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

bmcustodio commented 4 years ago

This makes me wonder whether people have been using gateway_invoke=true, because it doesn't seem to be working at all as of the latest couple releases. 🤨

alexellis commented 4 years ago

I'm not sure how many users update as frequently as Form3. I would have thought that I would have seen this though.

bmcustodio commented 4 years ago

@alexellis if you agree with the changes, could you please get merge it and tag?

alexellis commented 4 years ago

Thanks for raising this @bmcstdio

@jonatasbaldin this appears to be related to a patch you contributed around 3 months ago, please can you also take a look? https://github.com/openfaas/nats-queue-worker/commit/56df8a3bb77c6cc95446805a6aa46186e844ba66

alexellis commented 4 years ago

/close

alexellis commented 4 years ago

Thank you for the patch and for raising this @bmcstdio, let's take it through in #73 where I've been able to add some more guarding against unhandled errors.

openfaas / nats-queue-worker