default env to production

[maxcbc]

I just want to query the defaulting of env to 'development' in app.defaultConfiguration.

In a situation where verbose error messaging may expose sensitive information, there is a risk on information leakage from someone failing to explicitly set NODE_ENV=production.

I'm keen to see the arguments for it being 'development', my feeling is that it should be defaulted to 'production' and then if for some reason it hasn't been explicitly set so, the risk of information leakage is mitigated. Though its entirely likely I'm just being stupid.

EDIT:
Because it felt like a bit of bad manners to raise an issue without raising a PR to fix it see here: #3347

EDIT: On 'view cache' and more
Having read through:

• https://expressjs.com/en/advanced/best-practice-performance.html#set-nodeenv-to-production
• https://github.com/expressjs/express/blob/master/lib/application.js

The key performance benefit of setting NODE_ENV=production is that views are cached when using app.render. With the first link above stating an app performance boost of up to 'a factor of 3'.

Again I think there is benefit to defaulting to 'production' in this case, as by default we should want 'express' to be the most performant it can be, giving users the best possible experience straight out of the box.

[dougwilson]

Hi @maxcbc we can certainly discuss adding it to the next major version :) ! I don't 100% have insight to why the default was chosen, as this was before I was even a part of this project, but even by the time I became involved, the default was heavily engrained.

If this is changed, we'll need to find a champion to create blog posts, inform publications of the change, and get everyone to update their development workflows. In development, stacktraces / any helpful error message will be gone by default, but even weirder, no longer will editing your template files result in changes until the Express server is stopped and then started back up, so we should probably address what the impact to DX will be, exactly.

The biggest ask will be finding an advocate to help us change the workflows of our millions of developers, so I think we need to find someone for that, if we want to move forward.

I am a 👎 on the change, but it's a vote from @expressjs/express-tc , though I think it's not exactly clear on what yet to vote on since I'm not sure if you've fully evaluated the impact beyond just stacktraces. Can you flesh our your proposal a bit more, including adding the templating caching behavior change this will case and anything else across our entire ecosystem so we can better evaluate the ask?

[maxcbc]

Thanks for commenting, I really appreciate you taken the time. I'm inclined to agree with you on most points @dougwilson. This would actually be a big change, and should be a semver major sort of change if it were to happen. Particularly I agree this would be one of those things that would require a fair amount of communication with the community. I'm happy to play my part as much as I am able, but I'm not sure I have the depth of understanding to cover everything.

I'm still convinced that its a change worth doing for the reasons above, and I'm not one for not changing something just because its the status quo. That being said, I think further investigation is needed, I think its worthy of looking into, and I'd be eager to hear experience/views of how development experience may be impacted; particularly in reference to 'view cache' which is something I don't have much experience of personally.

I think as to stack traces etc and usefulness to developers, again I think this is a case of requiring communication at the point of a semver major update. Longer term it would be a relatively easy thing to work into 'getting started' documentation, to help even the most novice of users of express get used to how 'NODE_ENV' affects different aspects of the library.

[dougwilson]

Yea, like I said, we can certainly discuss adding it to the next major version :) ! I was trying to be open above to what would be involved, including the work that would need to be done to evaluate the impact prior to making the change. It's definitely OK if you're not able to champion these, but we would need to find someone before we can. There are a lot of modules / middleware around that are keying off the NODE_ENV / the env setting, so understanding what the effect of the changes will be here and determining what we need to work through is the first step before changing any code. You can see that for everything we are changing / removing between 4.x and 5.x we have an entire deprecation framework and lifecycle those are going through, and this should go through the same thing, including community outreach throughout the process.

[RomanovRoman]

May be we needs in options param?

var express = require('express');
var app = express({ env: process.env.NODE_ENV || 'production' });

[dougwilson]

@RomanovRoman here is how you do that in all versions of Express:

var express = require('express');
var app = express().set('env', process.env.NODE_ENV || 'production');

[RomanovRoman]

@dougwilson how it works for 'view cache' in this issue?

[dougwilson]

Hi @RomanovRoman I'm sorry, I don't understand what you're asking. Can you phrase it differently?

[RomanovRoman]

@dougwilson the issue is about "defaulting of env to 'development' in app.defaultConfiguration", isn't it?

IMHO behavior of

var app = express({ env: process.env.NODE_ENV || 'production' });

differs from

var app = express().set('env', process.env.NODE_ENV || 'production');

because application.js#L118

[dougwilson]

Hi @RomanovRoman I think I understand what you are saying now.

[dougwilson]

This issue got closed by the OP before a vote / discussion was able to occur. Reopening as the tracking discussion for this.

[dougwilson]

I just wanted to circle around to this hanging issue. I believe this was discussed and the general consensus was to change the default to production to prevent client stack traces by default, eventually removing the magic environment stuff. I can't find the threads yet, but just wanted to write down this as a note here.