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

nodejs12 (and nodejs14) templates only return error 500 on context.fail() #257

Closed
hrak opened this issue Apr 5, 2021 · 5 comments · Fixed by #262
Closed

nodejs12 (and nodejs14) templates only return error 500 on context.fail() #257

hrak opened this issue Apr 5, 2021 · 5 comments · Fixed by #262

Comments

@hrak
Copy link

hrak commented Apr 5, 2021

The nodejs templates currently return a 500 Internal Server Error in all cases (and do not output supplied result) on a call to context.fail()

F.e. on a return statement like this: return context.status(405).fail({"status": "method not allowed"}):

Expected Behaviour

> GET /function/jsonlint HTTP/1.1
> Host: openfaas.example.com
> User-Agent: curl/7.59.0
> Accept: */*
>
< HTTP/1.1 405 Method Not Allowed
< Content-Length: 32
< Content-Type: text/html; charset=utf-8
< Date: Mon, 05 Apr 2021 06:08:57 GMT
< Etag: W/"f-wdRP8Dr/E3KFbCgYVPRU4uHRW3w"
< Vary: Accept-Encoding
< X-Duration-Seconds: 0.007115
<
* Connection #0 to host openfaas.example.com left intact
{"status": "method not allowed"}

Current Behaviour

> GET /function/jsonlint HTTP/1.1
> Host: openfaas.example.com
> User-Agent: curl/7.59.0
> Accept: */*
>
< HTTP/1.1 500 Internal Server Error
< Content-Length: 15
< Content-Type: text/html; charset=utf-8
< Date: Mon, 05 Apr 2021 06:08:57 GMT
< Etag: W/"f-wdRP8Dr/E3KFbCgYVPRU4uHRW3w"
< Vary: Accept-Encoding
< X-Duration-Seconds: 0.007115
<
* Connection #0 to host openfaas.example.com left intact
[object Object]

Possible Solution

My guess is that the arguments on this line need to be reversed

Steps to Reproduce (for bugs)

Use the following handler.js and do a GET request on it.

'use strict'

module.exports = async (event, context) => {
  if(event.method == "POST") {
    const result = {
      'body': JSON.stringify(event.body),
      'content-type': event.headers["content-type"]
    }

    return context
      .status(200)
      .succeed(result)
  } else {
    return context.status(405).fail({"status": "method not allowed"})
  }
}

Context

Trying to return a 405 error on invalid method, but always getting a 500 Internal Server Error with [object Object] as body

Your Environment

  • Docker desktop 322 Mac OS X 10.14
  • using FaaS-netes on K3S
  • faas-cli 0.13.9
@alexellis
Copy link
Member

This is expected behaviour, but I can see why you are confused by it.

We'll take a look at it, but are open to taking a PR if you wanted to first propose how you think it should work?

Alex

@hrak
Copy link
Author

hrak commented Apr 5, 2021

Maybe i misunderstood, but i saw it more as a way to exit the handler in any non-OK state (not 2xx or 3xx), with the ability of returning a status code and message.

This is also the idea i got from this blog post, which actually has the exact return context.status(405).fail({"status": "method not allowed"}) line as in my "Steps to reproduce" above.

What it does now is return a 500 and [object Object], ignores the status, and logs the message to console.

@alexellis
Copy link
Member

Would you be open to taking a look into this and sending in a PR @hrak?

@hrak
Copy link
Author

hrak commented Apr 8, 2021

I can take a stab at it!

hrak added a commit to hrak/templates that referenced this issue Apr 13, 2021
alexellis added a commit that referenced this issue May 12, 2021
Tested with a 401 status before failing the message and saw
the code as expected.
Also tested with just calling fail, and saw the 500 code.

Closes #258 and Fixes: #257

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
alexellis added a commit that referenced this issue May 12, 2021
Tested with a 401 status before failing the message and saw
the code as expected.
Also tested with just calling fail, and saw the 500 code.

Closes #258 and Fixes: #257

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@alexellis
Copy link
Member

This is fixed by #262

I thought we'd waited long enough given how useful this could be for other users.

If you value our work and the project then please become a GitHub Sponsor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants