Skip to content
This repository has been archived by the owner on Jun 18, 2020. It is now read-only.

[WIP] Fix fail method #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[WIP] Fix fail method #5

wants to merge 2 commits into from

Conversation

kturcios
Copy link
Contributor

@kturcios kturcios commented Mar 4, 2019

The following PR will fix an issue where context.fail() did not return custom status codes even though context.status() was called.

Description

I modified the context.succeed() and context.fail() methods to return a default status code of 200 and 500, respectively.

When the callback method (cb) is invoked, it will check which status code to return by checking if it has been defined by the handler (i.e. this.statusCode !== undefined). Otherwise, it will return the default status code mentioned above.

Motivation and Context

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

Which issue(s) this PR fixes

Fixes #4

How Has This Been Tested?

I created a handler with the following snippet and got the expected status code and message.

module.exports = (event, context) => {
    context
        .status(501)
        .fail('An error occurred');
}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s

@derek derek bot added the new-contributor label Mar 4, 2019
@kturcios kturcios changed the title Fix fail method [WIP] Fix fail method Mar 4, 2019
Copy link

@tuct tuct left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for this one, I ended up using the succeed function for fail to be able to set the status code according to my needs.

Also I had to modify the template later in order to enable gzip, might create a pr soon which will expose express app for that purpose to the Handler on init only (this would also allow other tweaks to the express app).

@alexellis
Copy link
Member

I'm wondering if we should package a small unit test using mocha and a dev dependency?

@alexellis
Copy link
Member

alexellis commented Apr 29, 2019

👋 @tuct

👇 ?

Join Slack to connect with the community
https://docs.openfaas.com

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Context.fail does not support custom status codes
3 participants