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

Cannot set property 'cache-control' of undefined in io.js/v0.12 #2548

Closed
halfdan opened this issue Feb 15, 2015 · 6 comments
Closed

Cannot set property 'cache-control' of undefined in io.js/v0.12 #2548

halfdan opened this issue Feb 15, 2015 · 6 comments
Assignees
Labels

Comments

@halfdan
Copy link

halfdan commented Feb 15, 2015

Not exactly sure what is happening here but express.js is failing on io.js and v0.12 when used within Ghost: https://travis-ci.org/halfdan/Ghost/builds/50824137

TypeError: Cannot set property 'cache-control' of undefined
      at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:341:22)
      at ServerResponse.header (/home/travis/build/halfdan/Ghost/node_modules/express/lib/response.js:695:10)
      at ServerResponse.header (/home/travis/build/halfdan/Ghost/node_modules/express/lib/response.js:698:12)
      at Object.errors.error404 (/home/travis/build/halfdan/Ghost/core/server/errors/index.js:290:13)
      at Context.<anonymous> (/home/travis/build/halfdan/Ghost/core/test/unit/error_handling_spec.js:342:20)

Here's a piece of code that shows the issue:

var express = require('express');
var res = express.response;
res.set({'cache-header': 'foo'});

Above code works in v0.10 but not in v0.12. Express version is latest 4.11.2.

ref #2539, #2538

@halfdan
Copy link
Author

halfdan commented Feb 15, 2015

__proto__ is deprecated and probably not supported in either version anymore: https://github.com/strongloop/express/blob/master/lib/response.js#L29-L31

@dougwilson dougwilson self-assigned this Feb 15, 2015
@dougwilson
Copy link
Contributor

Hi! This is not an express bug, but simple a change between 0.10 and 0.12. express.response is just a subclass of the code Node.js require('http).ServerResponse.

As such, you must construct a new instance of the class to actually call methods on it. You should have been newing up a copy all this time, but as of 0.12 they added more stuff to the constructor so now you really need to new it up before you can interact with the headers.

As for that express.response export, even though you can certainly new it up and fix your issue, we don't even support you doing that; it's only there so you can attach your own methods to the Express response prototype, not to construct objects with.

This what you're doing, reduced down to Node.js core only. You can see it "works" in 0.10 (but of course, pollutes the global http response prototype), but not 0.12:

var http = require('http');
var res = http.ServerResponse;
res.setHeader('cache-header', 'foo');

@dougwilson
Copy link
Contributor

TL;DR, your test mocking was just fragile enough to notice a change in Node.js core. I recommend not using mocking.

@dougwilson
Copy link
Contributor

proto is deprecated and probably not supported in either version anymore

Um, __proto__ is part of the ES6 draft specification now... https://people.mozilla.org/~jorendorff/es6-draft.html#sec-__proto__-property-names-in-object-initializers But it's not relevant to this issue, which is that you need to var res = new express.response().

@lukesneeringer
Copy link

If you try to do var res = new express.response(), you get:

  - TypeError: express.response is not a function

@dougwilson
Copy link
Contributor

This is a really old thread from many months ago. The proper solution is the following:

$ node -pe 'function Mock(){}; Mock.__proto__ = require("express").request; new Mock()'
{}

@expressjs expressjs locked and limited conversation to collaborators Sep 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants