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

hrak commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

hrak commented 3 years ago

I can take a stab at it!

alexellis commented 3 years ago

This is fixed by https://github.com/openfaas/templates/pull/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

openfaas / templates