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

Setting the status code with res.status has no effect #1429

Closed
3 tasks done
svozza opened this issue Aug 2, 2017 · 3 comments
Closed
3 tasks done

Setting the status code with res.status has no effect #1429

svozza opened this issue Aug 2, 2017 · 3 comments
Assignees
Labels

Comments

@svozza
Copy link
Contributor

svozza commented Aug 2, 2017

  • Used appropriate template for the issue type
  • Searched both open and closed issues for duplicates of this issue
  • Title adequately and concisely reflects the feature or the bug

Bug Report

Restify Version

5.0.1

Node.js Version

Node 8

Expected behaviour

When using res.status in a route, I expect the status code set there to be the one returned when
hitting the route.

Actual behaviour

The route always returns a 200.

Repro case

Here's the test I have in my branch:

test('GH-1429: setting code with res.status not respected', function (t) {
    SERVER.get('/404', function (req, res, next) {
        res.status(404);
        res.send(null);
    });

    CLIENT.get(join(LOCALHOST, '/404'), function (err, _, res) {
        t.equal(res.statusCode, 404); // always returns a 200
        t.end();
    });
});

Cause

The offending code is here:

https://github.com/restify/node-restify/blob/5.x/lib/response.js#L307
https://github.com/restify/node-restify/blob/5.x/lib/response.js#L311

On line 307 if code is undefined it gets set to 200 and then on line 311 that overwrites self.statusCode even if it has already been set.

Are you willing and able to fix this?

Yes. In fact, I've already fixed it on my fork.

@kokarn
Copy link

kokarn commented Aug 4, 2017

Encountering this issue as well

@retrohacker retrohacker self-assigned this Aug 7, 2017
@retrohacker retrohacker added the Bug label Aug 7, 2017
@retrohacker
Copy link
Member

This was me 😞

@DonutEspresso
Copy link
Member

This was just merged in - think we are good to cut a release for this!

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

No branches or pull requests

4 participants