Node.js with Express integration?

[Steven Bull]

Hi there,

I'm super excited to try out Passenger's support for Node.js! I'm trying to understand the story at https://github.com/phusion/passenger/wiki/Node.js, however, I've got several apps built using Express (http://expressjs.com/), and I'm not sure how to integrate with that. FYI, I'm using Nginx.

According to the docs, it looks like I have to make some changes to how listening for requests works, as PhusionPassenger doesn't follow the traditional Node.js app flow. Normally, I would do something like http.createServer(app).listen(port), but now I'm not in control of the port binding. It appears that I will need to deal with the socket that is passed to the callback for "request" events directly somehow.

My initial thinking was that I might be able to do something along the lines of the following:

server = http.createServer(app);

PhusionPassenger.on('request', function(headers, socket) {

  server.listen(socket);

});

But as I think about it, I'm guessing that this won't quite work. I don't believe it's appropriate to call server.listen() many times like that, and it seems really wrong to create a new server object for each request event. Furthermore, I suspect that the socket isn't quite in the state that the server expects it to be in - apparently you've already read the headers off of the socket, as you pass them in to the callback as a parameter.

Is there a way to make this work? Am I missing something about how to make PhusionPassenger play nicely with Express (or http.Server, for that matter)?

Thanks!

Steve

[Hongli Lai]

According to http://expressjs.com/api.html#app.listen, the app object
returned by express() is just a Function which is compatible with the
httpServer callback. The function has therefore the following
signature:

function (request, response) { }

So we'll need a layer which:
- listens on the PhusionPassenger 'request' event
- turns 'headers' into an http.ServerRequest object
- implements a response object with an API compatible with
http.ServerResponse, but which actually writes to 'socket'
- calls the app object with the given request and response objects

> --
> You received this message because you are subscribed to the Google Groups
> "Phusion Passenger Discussions" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to phusion-passen...@googlegroups.com.
> To post to this group, send email to phusion-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/phusion-passenger.
> For more options, visit https://groups.google.com/groups/opt_out.



--
Phusion | Ruby & Rails deployment, scaling and tuning solutions

Web: http://www.phusion.nl/
E-mail: in...@phusion.nl
Chamber of commerce no: 08173483 (The Netherlands)

[Hongli Lai]

I've taken a look at how httpServer is implemented. It looks doable,
but I'm not sure how stable their API is. It seems that in the past 6
months they've changed the internals yet again. Still, I'll give it a
try.

[Hongli Lai]

I think I've succeeded in emulating the httpServer objects. See this
example: https://gist.github.com/FooBarWidget/6525478

Could you pass the created 'req' and 'res' objects to your Express.js
app object and check whether this works?

[Steven Bull]

I've done a little bit of testing of this with one of my apps, and so far, it appears to be working.

Let me make sure that I got your intent correct though. Here's what the bottom of my passenger_node.js file looks like now:

PhusionPassenger.on('request', function(headers, socket) {

  var req = createIncomingMessage(headers, socket);

  var res = new http.ServerResponse(req);

  res.assignSocket(socket);

  res.shouldKeepAlive = false;

  res.once('finish', function() {

    socket.destroySoon();

    });

  app(req, res);

});

Did I get it right here? Did you intend for me to keep the res.shouldKeepAlive and res.once() lines, and replace the res.writeHead(), res.write(), res.end() lines?

I'll be working on more of my apps in the next few days, so I'll let you know if I run into any problems. But please let check that I'm following this correctly. Thanks for the great quick help!

p.s.: I love the APT repository also. Many thanks.

[Hongli Lai]

Yes, you did that correctly. Everything before the first
res.writeHead() was intended to be setup code.

[Steven Bull]

Awesome. As a follow-up question, do you expect that you'll be able to incorporate any of this into your PhusionPassenger library for Node? Seems like it would be easier for lots of people for the Passenger integration to be something along the lines of...

Replace the following line in your Node.js script:

http.createServer(app).listen(...)

with the following:

PhusionPassenger.createServer(app);

Or something similar. And then you maybe wouldn't need to get into as much detail about the inverted control flow that Passenger introduces, at least for a quick-start guide / simple use case. Thoughts?

[Hongli Lai]

Yes, that's the plan. We'll provide a low-level API that exposes the
CGI headers and the raw socket, as well as a higher-level API that can
wrap httpServer or at least provide httpServer-compatible request and
response objects. Unfortunately I think I'll have to break the current
API to make this happen efficiently.

[Steven Bull]

Hi Hongli,

I'm running into trouble with some parts of my app. The problem I'm noticing is that routes that I have configured via Express's app.put(...) are no longer working (requests using the http method PUT). The symptom is that req.body is null when handling those requests.

I've got a bit more digging in on this to do - and perhaps creation of a simple test case - but if you're able to figure out if there's something in the Passenger code that might be breaking my PUT requests, let me know.

Thanks again,

Steve

[Hongli Lai]

Is there supposed to be a 'request.body' property? It's not documented
in http://nodejs.org/api/http.html. Is it non-standard and/or an
implementation detail? What is 'body' supposed to contain? A string
with the full body data? A filename to a file containing the data? A
file descriptor?

From what I read http://nodejs.org/api/http.html, it would appear that
the API expects apps to read the body from the socket.

[Puneet T]

Hi Hongli,

req.body is part of Express framework, and it not native to node.js.

Here is the documentation for req.body:

http://expressjs.com/api.html#req.body

The req.body is an object containing the parsed request body.

It requires bodyParser middleware:

http://expressjs.com/api.html#bodyParser

--
Puneet

[Steven Bull]

Hi Hongli,

You're right, 'request.body' is something that Express creates. But it looks like there's an underlying problem preventing Express from getting it. And this is not just a problem with PUT requests, but with standard POST requests as well.

I've done some bare-bones testing of a request callback that tries to read from the IncomingMessage stream, and it looks like it's never able to read any data. (I also discovered some issues with your gist https://gist.github.com/FooBarWidget/6525478, on which I commented).

Anyway, here's a simple app (without any framework dependencies) that works outside of PhusionPassenger:

var http = require('http');

var PORT = process.env.PORT || 3456;

var app = function(req, res){

  console.log(JSON.stringify(req.headers));

  var reqBody = '';

  req.setEncoding('utf8');

  req.on('data', function(chunk){

    reqBody += chunk;

  });

  req.on('end', function(){

    var resBody = 'Got '+reqBody.length+' bytes of request body:\n'+reqBody+'\n';

    console.log(resBody);

    res.writeHead(200, { 'Content-Length': resBody.length, 'Content-Type': 'text/plain' });

    res.write(resBody);

    res.end();

  });

};

http.createServer(app).listen(PORT, function(){

  console.log('Express server listening on port ' + PORT);

});

You can test this using:

curl -i -d 'foo=bar' http://localhost:3456/

However, if I try to use this with PhusionPassenger and call app(req,res), the request just hangs without ever giving a response. It looks like the request stream is never issuing any events ('data', 'end'). I've tried to look at other events also ('error', 'close', 'readable'), and tried to use req.read() as well, but I can't get anything. It seems as though the stream isn't behaving properly.

Are you able to take a look at this? Thanks so much!

[Hongli Lai]

I had taken some days off. I'm resuming work on this today.

[Hongli Lai]

Ok, I think I got it working. Apparently Express.js expects
socket-like events on the request objects, but these events are not
documented in the Node.js API. Please try my updated code at
https://gist.github.com/FooBarWidget/6525478

[Hongli Lai]

Also, I discovered a bug in Phusion Passenger's Node.js support code,
which I've fixed in git master. Please try running this against git
master.

[Steven Bull]

Hello again, Hongli :)  I assumed you were out for a bit, so thanks for coming back!

I've tried the new code from the gist with the master branch from github, and here are my findings. I'm using the same simple app from my previous response (independent of Express) for this.

First off, it's working much better.

However, I've found a few issues, I believe.

1. 'end' event on GET requests: It appears that for GET requests, the req is not emitting an 'end' event, whereas it does when using http.createServer().

2. HTTPS detection: There are still some data available in the PhusionPassenger headers that don't appear to be transferred to the req. In particular, I can see that the headers contain "HTTPS":"on" when used over https:, but I can't find that information in the req. I've previously used the following nginx config line to make this available to my apps:

proxy_set_header X-Forwarded-Proto $scheme;

I'm wondering if that would be the best way to do this for Passenger, but it seems like something might be implied by the description of http.IncomingMessage.socket (http://nodejs.org/api/http.html#http_message_socket) also.

3. IP Address / port not available: I'm not able to retrieve the client's remote IP address from the req. It's present in the headers (REMOTE_ADDR, REMOTE_PORT), but it's not being transferred to the req. I think the way to get this from the req would be either req.connection.remoteAddress or req.socket.remoteAddress (& remotePort). req.connection.remoteAddress is mentioned on StackOverflow a few times, but I don't see documentation for it in http.IncomingMessage (though socket is there). I believe nginx had set the X-Forwarded-For header when set up to proxy to apps, and again I'm wondering if that would be best here.

That's what I've discovered for now. I'll be paying attention for your updates. Thanks!

[Roberto Hidalgo]

Hello to both! I've been trying passenger 4.0.19 and migrating some applications to it, using the gist Hongli posted, but have no idea how to have both /socket.io/socket.io.js or the socket proper handled with it.

I've tried:

    var express = require('express');

    var app = express();

    var server = require('http').createServer(app);

    var io = require('socket.io').listen(server);

    

    // all the gist code

    

    PhusionPassenger.on('request', function(headers, socket){

        var req = createIncomingMessage(headers, socket);

        var res = createServerResponse(req);

        app(req,res);

    });

But, so far, have not been able to make it work. Any ideas? Is this even possible with Passenger?

Thanks!

[Steven Bull]

Hi Roberto, your question is a bit confusing to me - I'm not sure what you are doing with socket.io, or if you are just using Express?

As I mentioned, there's currently a problem with GET requests and the gist code (they don't ever "end"), and I think Hongli fixed some things in the master branch from github that aren't in the 4.0.19 release, so you may have to install from source instead of the released package.

Anyway, I think this code might work better for you with Passenger:

var express = require('express');

var app = express();

// DON'T do http.createServer(), not sure why you need socket.io?

// all the gist code

PhusionPassenger.on('request', function(headers, socket){

  var req = createIncomingMessage(headers, socket);

  var res = createServerResponse(req);

  app(req,res);

});

[Steven Bull]

To fix the problem with GET events, the following appears to work:

PhusionPassenger.on('request', function(headers, socket) {

  var req = createIncomingMessage(headers, socket);

  var res = createServerResponse(req);

  app(req, res);

  if (req.method === 'GET' && req.read() === null) {

    req.emit('end');

  }

});

[Hongli Lai]

On Thu, Sep 26, 2013 at 11:29 AM, Steven Bull <steve...@gmail.com> wrote:
> Hello again, Hongli :) I assumed you were out for a bit, so thanks for
> coming back!
>
> I've tried the new code from the gist with the master branch from github,
> and here are my findings. I'm using the same simple app from my previous
> response (independent of Express) for this.
>
> First off, it's working much better.
>
> However, I've found a few issues, I believe.
>
> 1. 'end' event on GET requests: It appears that for GET requests, the req is
> not emitting an 'end' event, whereas it does when using http.createServer().

I am a little unsure about what to do with this one. The reason why
you don't get an 'end' event is because the end simply hasn't been
reached! If you make a GET request with curl, then curl does not send
a Content-Length header, but does not half-close the socket either.
GET requests *may* contain a body (see
http://stackoverflow.com/questions/978061/http-get-with-request-body),
especially in the context of WebSockets, so we can't simply fake an
end event either upon receiving a GET request.

How does Node typically handle WebSockets? Do people use HttpServer for that?

> 2. HTTPS detection: There are still some data available in the
> PhusionPassenger headers that don't appear to be transferred to the req. In
> particular, I can see that the headers contain "HTTPS":"on" when used over
> https:, but I can't find that information in the req. I've previously used
> the following nginx config line to make this available to my apps:
>
> proxy_set_header X-Forwarded-Proto $scheme;
>
> I'm wondering if that would be the best way to do this for Passenger, but it
> seems like something might be implied by the description of
> http.IncomingMessage.socket
> (http://nodejs.org/api/http.html#http_message_socket) also.

How do Node users currently handle HTTPS scenarios? Is checking for
X-Forwarded-Proto the defacto standard way?

> 3. IP Address / port not available: I'm not able to retrieve the client's
> remote IP address from the req. It's present in the headers (REMOTE_ADDR,
> REMOTE_PORT), but it's not being transferred to the req. I think the way to
> get this from the req would be either req.connection.remoteAddress or
> req.socket.remoteAddress (& remotePort). req.connection.remoteAddress is
> mentioned on StackOverflow a few times, but I don't see documentation for it
> in http.IncomingMessage (though socket is there). I believe nginx had set
> the X-Forwarded-For header when set up to proxy to apps, and again I'm
> wondering if that would be best here.

I've updated the gist and am setting
req.connection.remote{Socket,Address} for now:
https://gist.github.com/FooBarWidget/6525478

[Steven Bull]

Hey Hongli,

I'm not as experienced in many of these issues as you're hoping, so what I'm saying now is mostly based on talking with other people or googling. I just wanted to point out those last 3 issues, as they'll probably become important somewhere down the road plug-and-play swapping of http.createServer() with PhusionPassenger.

1. GET requests hanging: As far as WebSockets go, I believe sockets.io is pretty standard (http://socket.io/#how-to-use). Additionally, when I used an express app as opposed to my simple demo app, GET requests were not hanging. So express must workaround this somehow, or perhaps they just disallow bodies for GET requests and socket.io does other interception of the request handling if it is in fact a WebSocket request?

2. HTTPS detection: http://stackoverflow.com/questions/10348906/how-to-know-if-a-request-is-http-or-https-in-node-js

I'm not actually using this yet in nodejs, but in Rails-world, I've used request.ssl? often (http://rack.rubyforge.org/doc/Rack/Request.html#method-i-ssl-3F), as is probably used in config.force_ssl also.

3. IP address: Again, I'm not currently using this, but your update to the gist looks like it should suffice.

Anyway, for my present purposes, Passenger appears to be working well now with Express. Of course, I'll let you know if I discover any issues after using it for a while. Thanks!

[Hongli Lai]

I appreciate the feedback in any case. :)

Today I found out that Node's HTTP parser simulates an 'end' event if
the GET request does not have an Upgrade header. I've updated the gist
to do the same thing. I'm now also setting X-Forwarded-Proto and
X-Forwarded-For. https://gist.github.com/FooBarWidget/6525478

I'll test things with socket.io and Express as well.

On Sat, Oct 5, 2013 at 10:57 PM, Steven Bull <steve...@gmail.com> wrote:
> Hey Hongli,
>
> I'm not as experienced in many of these issues as you're hoping, so what I'm
> saying now is mostly based on talking with other people or googling. I just
> wanted to point out those last 3 issues, as they'll probably become
> important somewhere down the road plug-and-play swapping of
> http.createServer() with PhusionPassenger.
>
> 1. GET requests hanging: As far as WebSockets go, I believe sockets.io is
> pretty standard (http://socket.io/#how-to-use). Additionally, when I used an
> express app as opposed to my simple demo app, GET requests were not hanging.
> So express must workaround this somehow, or perhaps they just disallow
> bodies for GET requests and socket.io does other interception of the request
> handling if it is in fact a WebSocket request?
>
> 2. HTTPS detection:
> http://stackoverflow.com/questions/10348906/how-to-know-if-a-request-is-http-or-https-in-node-js
> I'm not actually using this yet in nodejs, but in Rails-world, I've used
> request.ssl? often
> (http://rack.rubyforge.org/doc/Rack/Request.html#method-i-ssl-3F), as is
> probably used in config.force_ssl also.
>
> 3. IP address: Again, I'm not currently using this, but your update to the
> gist looks like it should suffice.
>
> Anyway, for my present purposes, Passenger appears to be working well now
> with Express. Of course, I'll let you know if I discover any issues after
> using it for a while. Thanks!
>

[Hongli Lai]

Success! I've been able to get socket.io working on Phusion Passenger.

I've integrated the code in the gist into Phusion Passenger itself.
Phusion Passenger now also emulates the HttpServer API more closely.
Instead of calling our custom API, like this...

PhusionPassenger.on('request', ...)

...you now just create a regular HttpServer object, and tell
PhusionPassenger to use it.

For example, here's how a regular Node.js app might look like:

var server = require('http').createServer(handler);
server.listen(3000);

function handler(req, res) {
....
}

To make this app Phusion Passenger compatible, you just need to add
one line. You need to tell Phusion Passenger to use the HttpServer
object, right before you listen, like this:

var server = require('http').createServer(handler);
server = PhusionPassenger.use(server);
...rest of the code here...

This change has been commited into commit f941fd3.

Since this approach is much more user friendly, the old Phusion
Passenger API is no longer supported.

[Hongli Lai]

And now, PhusionPassenger.use() supports Connect.js app objects as well.

On Wed, Oct 9, 2013 at 10:52 PM, Hongli Lai <hon...@phusion.nl> wrote:
> Success! I've been able to get socket.io working on Phusion Passenger.
>
> I've integrated the code in the gist into Phusion Passenger itself.
> Phusion Passenger now also emulates the HttpServer API more closely.
> Instead of calling our custom API, like this...
>
> PhusionPassenger.on('request', ...)
>
> ...you now just create a regular HttpServer object, and tell
> PhusionPassenger to use it.
>
> For example, here's how a regular Node.js app might look like:
>
> var server = require('http').createServer(handler);
> server.listen(3000);
>
> function handler(req, res) {
> ....
> }
>
> To make this app Phusion Passenger compatible, you just need to add
> one line. You need to tell Phusion Passenger to use the HttpServer
> object, right before you listen, like this:
>
> var server = require('http').createServer(handler);
> server = PhusionPassenger.use(server);
> ...rest of the code here...
>
> This change has been commited into commit f941fd3.
>
> Since this approach is much more user friendly, the old Phusion
> Passenger API is no longer supported.

[Hongli Lai]

It looks like Express app objects work correctly as well.

Testers would be much appreciated.

[Steven Bull]

My initial testing looks good.

Clever how you overwrote listen(), and ignore everything but the callback.

One thing - the typical "listen" callback goes something like this:

http.createServer(app).listen(PORT, function(){

  console.log('Express server listening on port ' + PORT);

});

And now, that's being spit out in my log file with every request instead of just on startup. :(

Brainstorming a few ideas:

- Maybe you could make the ".listen()" call optional. I can't tell why from looking at the code, but when I do PhusionPassenger.use(http.createServer(app)) without calling .listen(...) on it, it never picks up. Perhaps there's something special watching for the 'listening' event? But it would be great if there was no requirement to call .listen() when using PhusionPassenger, as it kind-of doesn't make sense (the port isn't respected etc.).

- When .listen(..., callback) is used, maybe you should just invoke the callback once, immediately, instead of attaching it to the 'request' event? It seems like that would be closer to the expected behavior.

- I'm thinking that I'll do a sort of switch like this in my app.js:

var server = http.createServer(app);

if (typeof PhusionPassenger === 'undefined') {

  server.listen(PORT, function(){

    console.log('Express server listening on port ' + PORT);

  });

}

else {

  PhusionPassenger.use(server).listen(); // prefer without the .listen()

}

and then just "require('./app');" in passenger_node.js. So that leads me to my next question - is it possible for you to use app.js itself instead of requiring passenger_node.js? Or perhaps use app.js if passenger_node.js doesn't exist?

And then finally, if you could use app.js straightaway, it would be awesome if you could somehow magically overwrite http.createServer() = ZERO APP CONFIG TO MAKE PASSENGER WORK!

Actually, based on this thought process and on node's description of module loading/caching (http://nodejs.org/api/modules.html#modules_caching), I did the following in passenger_node.js:

var http = require('http');

http.createServerWithoutPassenger = http.createServer;

http.createServer = function(requestListener){

  return PhusionPassenger.use(http.createServerWithoutPassenger(requestListener));

};

require('./app');

... and it worked! So if you could incorporate that into your node-loader.js, it would be wonderful... absolutely no app code changes required to switch from non-passenger to passenger, only web server config changes!?!?!?!?! Please please please!

[Hongli Lai]

On Thu, Oct 10, 2013 at 11:03 AM, Steven Bull <steve...@gmail.com> wrote:
> My initial testing looks good.
>
> Clever how you overwrote listen(), and ignore everything but the callback.
>
> One thing - the typical "listen" callback goes something like this:
>
> http.createServer(app).listen(PORT, function(){
> console.log('Express server listening on port ' + PORT);
> });
>
> And now, that's being spit out in my log file with every request instead of
> just on startup. :(

Oops, I misunderstood the purpose of the listen callback. I'll have this fixed.

> - Maybe you could make the ".listen()" call optional. I can't tell why from
> looking at the code, but when I do
> PhusionPassenger.use(http.createServer(app)) without calling .listen(...) on
> it, it never picks up. Perhaps there's something special watching for the
> 'listening' event? But it would be great if there was no requirement to call
> .listen() when using PhusionPassenger, as it kind-of doesn't make sense (the
> port isn't respected etc.).

I believe it's a good idea to keep the listen() call, but ignore the
port number. Lots of apps have a callback on the listen call. You want
that code to work with as few changes as possible.

If I make the listen() call optional, the developer will have to
change some lines in their code to make the listen callback be called
properly in case it's ran in Phusion Passenger. If I make the listen
call mandatory, then the developer only has to copy-paste one line.

The second reason why I think an explicit listen() is a good idea, is
because some apps may want to perform initialization before listening.
This initialization may take some time and may be non-blocking. For
example, reading files from disk, or querying configuration from a
remote server. By explicitly calling listen(), Phusion Passenger will
better know when the app is really done initializing, so that the
startup timeout feature can work properly. Suppose that the app tries
to query config from a remote server during startup, but that server
is slow or isn't responding. Phusion Passenger will be able to
determine that the app has failed to start within the timeout, and
will kill it.

> - I'm thinking that I'll do a sort of switch like this in my app.js:
>
> var server = http.createServer(app);
> if (typeof PhusionPassenger === 'undefined') {
> server.listen(PORT, function(){
> console.log('Express server listening on port ' + PORT);
> });
> }
> else {
> PhusionPassenger.use(server).listen(); // prefer without the .listen()
> }
>
>
> and then just "require('./app');" in passenger_node.js. So that leads me to
> my next question - is it possible for you to use app.js itself instead of
> requiring passenger_node.js? Or perhaps use app.js if passenger_node.js
> doesn't exist?

It is currently not possible. Is the name "app.js" a convention in the
Node community? Where is this name used?

> And then finally, if you could use app.js straightaway, it would be awesome
> if you could somehow magically overwrite http.createServer() = ZERO APP
> CONFIG TO MAKE PASSENGER WORK!

That wouldn't be a good idea. The app may want to spawn a separate
http server, outside Passenger's control, for whatever reason. Some
libraries may create an httpServer under the hood. It's better to keep
it explicit, for maximum compatibility.

[Steven Bull]

Hey Hongli,

This feels like a debate that I'm likely to lose, but I'll go ahead with it anyway.

First, fair enough with the need to call .listen(). You're right, there are likely fewer code modifications required that way, and it's probably a matter of personal coding style preference to be able to have a different startup method.

Now to the bigger debates:

1. app.js vs. passenger_node.js

"app.js" is generated by some frameworks (Express) as the standard startup file. In most of the recent node articles, app.js is assumed to be the name of your startup file. However, there is also some use of server.js as well. More particularly, the "npm start" command assumes that server.js is the startup file, unless package.json contains a different value for the scripts.start key (https://npmjs.org/doc/misc/npm-scripts.html#DEFAULT-VALUES).

A nice approach for this might be to try to look at the scripts.start key in package.json and attempt to figure out what it tells you to do, then do some default ordering of files to load based on existence (server.js if it exists, then app.js if it exists). But assuming that's too involved, would it be possible to have a web server configuration setting to specify the startup script (e.g. "passenger_nodejs_load app.js")? I'm just aiming for web-server-only configuration.

2. Overwriting http.createServer

My approach would be this: make it stupid-simple for the 99% use case, and the other 1% possible. I'm guessing that 99% of the people who are trying to use Passenger to launch their node apps will be using exactly 1 httpServer in their app, created by http.createServer, and not have any libraries or other code in use that calls http.createServer(). So for the vast majority of users of Node with Passenger, overwriting http.createServer and enabling zero changes to the app code would be perfect.

Then, for the 1% of obscure use cases, you could provide instructions/documentation to call "PhusionPassenger.restoreHttpServer();" as the first line of your script to put http.createServer back to its original value. In fact, you might even want to do this automatically on the first call of the overwritten http.createServer() anyway (http.createServer = http.createServerWithoutPassenger). I believe this would address any of the compatibility concerns.

With that approach, it seems to me that most users would not need to do any code changes to make their node apps run with Passenger, though a few advanced users might need to make some simple calls (PhusionPassenger.restoreHttpServer() and PhusionPassenger.use()). Without this approach, every user of Passenger will need to change their app code.

Finally, if this isn't an option for default functionality, would it be possible to make this a web server config setting? (passenger_nodejs_hijack_http_create_server on;)

Well, there's my argument. Thanks!

[Tinco Andringa]

Hey Steven,

I think there's much to be said for both your points, so you might not
lose that debate, there's always a chance ;)

> 1. app.js vs. passenger_node.js
> ... But assuming that's too involved, would

> it be possible to have a web server configuration setting to specify the
> startup script (e.g. "passenger_nodejs_load app.js")? I'm just aiming for
> web-server-only configuration.

I would definitely support the idea of a sane default for that name.
The ruby world has standardized on 'config.ru' and that makes
deployment really easy. I think Hongli chose 'passenger_node.js as a
file name because he was unaware of any convention, we have this
situation for Python as well. We're a bit wary of adding nginx
configuration, but I think the web-server-only configuration is a very
valid point. Perhaps to prevent a whole bunch of passenger_*_load
settings, perhaps one that works for all languages. It could be
something like passenger_startup_script. Based on the extension it
could determine what kind of app it is.

> 2. Overwriting http.createServer
>
> My approach would be this: make it stupid-simple for the 99% use case, and
> the other 1% possible. I'm guessing that 99% of the people who are trying to
> use Passenger to launch their node apps will be using exactly 1 httpServer
> in their app, created by http.createServer, and not have any libraries or
> other code in use that calls http.createServer(). So for the vast majority
> of users of Node with Passenger, overwriting http.createServer and enabling
> zero changes to the app code would be perfect.
>
> Then, for the 1% of obscure use cases, you could provide
> instructions/documentation to call "PhusionPassenger.restoreHttpServer();"
> as the first line of your script to put http.createServer back to its
> original value.

This one will receive more resistance I think. The 1% will likely feel
seriously violated that some core library feature has been monkey
patched in some weird way. Even though it might be thoroughly
documented, people don't actually read documentation so it's
guaranteed they will fail at first. Because of the nature of the
patch, it will be pretty hard to find out what's going on. Though
eventually a google for "Passenger http.createServer doesn't work"
will yield the SO answer ;)

> In fact, you might even want to do this automatically on the
> first call of the overwritten http.createServer() anyway (http.createServer
> = http.createServerWithoutPassenger). I believe this would address any of
> the compatibility concerns.

This sounds rather hacky and fragile, but might actually work. We
probably need a good discussion on how much we value deployment ease
over engineering integrity, besides an analysis of in which cases this
could horribly back fire :)

Thanks for contributing to Passenger!

> --
> You received this message because you are subscribed to the Google Groups
> "Phusion Passenger Discussions" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to phusion-passen...@googlegroups.com.
> To post to this group, send email to phusion-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/phusion-passenger.
> For more options, visit https://groups.google.com/groups/opt_out.

[Steven Bull]

Hey Tinco,

A few points to consider:

- Unless you can overwrite http.createServer, I don't see much value in being able to configure the startup script filename. The whole goal was to be able to set up everything just in the web server config without touching app code, as that would make deployment much easier. And (given that people don't read documentation), it would be easier to get them to just update their nginx config (which they have to do anyway to make Passenger work) than update both the nginx config and the app code itself (and, at least in bigger companies, those two jobs could be different teams that require coordination etc.).

- I highly suspect that the people trying to use Passenger for an app that does something other than use just one http.createServer() are much more motivated to read the documentation and understand how Passenger modifies things than the other 99%. Passenger will already be taking over the typical port-listening ability of http.createServer, so the 1% using more complicated apps will probably be quite concerned about how they need to modify their apps anyway.

- It might be fairly straightforward to spit out warning or error messages in the log files if the Passenger-hijacked http.createServer() is called more than once by an app, and tell people what they need to do:

http.createServer = function(requestListener){

  http.createServer = function(rl2){

    console.log("WARNING: Your app is using PhusionPassenger and calling http.createServer multiple times. To do this correctly, please see the documentation at https://groups.google.com/forum/#!topic/phusion-passenger/sZ4SjU8ypwc.");

    return http.createServerWithoutPassenger(rl2);

  };

  return PhusionPassenger.use(http.createServerWithoutPassenger(requestListener));

};

[Hongli Lai]

You guys have already said everything I wanted to discuss. :)

I can make Phusion Passenger check for both app.js and server.js. No
configuration option needed.

I also think that showing a warning or error on the second
http.createServer() is a good idea. It should be obvious enough so
that the magic does not backfire. I'm not entirely sure whether it
will work in case the user is using Connect, Express or Socket.io.
I'll dig into this.

[Hongli Lai]

Wow, that actually worked much better than expected. Plain HttpServer,
Connect, Express and Socket.io all worked out of the box.

Instead of overriding http.createServer(), I override
http.Server.prototype.listen. The first call installs that Server
object as the Passenger request handler, the second raises an
exception and gives the user a helpful message. I call this the
"auto-install mode". It is on by default.

If users want to designate a specific HttpServer object as the request
handler, then they can disable auto-install mode by calling the
following before any HttpServer has been created:

PhusionPassenger.configure({ autoInstall: false });

They are then expected to call listen() with 'passenger' as the port "number":

http.createServer(app).listen('passenger');

This change has been committed as f690503. I'll work on the
app.js/server.js thing next.

[Steven Bull]

This is looking fantastic.

Just FYI, I've noticed a couple more places that you could check for startup files, thought I don't believe they are very widely used:

web.js: The only place I've seen this is in Heroku's documentation (https://devcenter.heroku.com/articles/getting-started-with-nodejs#declare-process-types-with-procfile), probably to keep things clearer in their separation of "web" dynos and "worker" dynos. But since it's in Heroku's docs, that convention could get adopted by more people.

package.json's "main" property: I saw this in nodemon's usage documentation (https://github.com/remy/nodemon#usage), but it seems to be a bad way to go, as "It's highly advised that requiring the main file NOT generate any side-effects. For instance, requiring the main file should NOT start up an HTTP server or connect to database." (http://package.json.nodejitsu.com/)

Thanks much!

[Hongli Lai]

After reading about server.js, I don't think using it is a good idea. 'Npm start' appears to be meant for starting servers/daemons. Since passenger initiates port binding, using server.js would not make sense.

The main property is used for require(), so I don't think Passenger should use it.

Heroku does not use web.js as a convention. It was mainly used as an example.

So that leaves app.js as the best candidate.

Sent from my Android phone.

--

[Hongli Lai]

Since commit 1929d80, app.js is used as the startup file.

[Garret Noling]

Been playing with this (latest passenger from git), however, it seems to have a problem when the response is over 64k. For instance:

var connect = require('connect')
  , http = require('http');

var app = connect()
  .use(connect.favicon())
  .use(connect.logger('dev'))
  .use(connect.static('public'))
  .use(connect.directory('public'))
  .use(connect.cookieParser())
  .use(connect.session({ secret: 'my secret here' }))
  .use(function(req, res){
    res.end('Hello from Connect!\n');
  });

http.createServer(app).listen(3000);

-OR-

var http = require('http');

var fs = require('fs');
var index = fs.readFileSync('public/vendor.js');

http.createServer(function (req, res) {
  res.writeHead(200, {'Content-Type': 'text/plain'});
  res.end(index);
}).listen(9615);

Both of these stall when servering a rather large javascript file I have named vendor.js (752k). When ran directly through node, they serve the file fine. When ran through passenger (through either mod_passenger or passenger standalone/nginx), they cut off at about 64k and the browser remains spinning waiting for the rest indefinitely. Smaller files/responses finish without issue.

[Steven Bull]

Hey Garret,

I'm not really qualified to make this determination, but I'm guessing this would best be served by starting a new topic for your issue. It seems fairly independent from the context of this thread so far, and would have a better title. But Hongli or Tinco would have more authority than me.

[Garret Noling]

Possibly, although my boiled-down examples are a result of my trying to use Node.js with Express specifically, as the topic suggests (it's also how I came to find this topic), which I experience the same behaviour with. My reply was a result of my findings when debugging the issue.

The boiling down for my example code simply brings it to connect, which Express is built on, and where my original issue resides. ;)

-Garret

--
You received this message because you are subscribed to a topic in the Google Groups "Phusion Passenger Discussions" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/phusion-passenger/sZ4SjU8ypwc/unsubscribe.
To unsubscribe from this group and all its topics, send an email to phusion-passen...@googlegroups.com.

[Hongli Lai]

Well, this topic has become a general discussion topic about Node
compatibility so I don't mind continuing the discussion here. :)

Garret, thanks for reporting this, but I can't reproduce your problem.
This is what I've tried:

var http = require('http');
var fs = require('fs');

var index = '';
for (var i = 0; i < 1024 * 1024; i++) {
index += 'x';

}

http.createServer(function (req, res) {
res.writeHead(200, {'Content-Type': 'text/plain'});
res.end(index);

}).listen(8000);

It sends a 1 MB response, and it works fine here.

Are you using the Nginx variant? Could you set passenger_log_level to
3, make a request which stalls, and send me the Nginx error log file?

By the way, Phusion Passenger performs static file acceleration. So if
there is a vendor.js in your public directory, and you access
/vendor.js, then Phusion Passenger serves that file directly from the
web server, not from Node.

[Garret Noling]

Sorry, looks like I got a little confused with my sample sets and wrote
an incorrect report! My bad.

The problem only happens when connect (or express, which is on top of
connect) is used. Your example made me curious, so I added a route to a
skeleton express app like this:

app.get('/1mb', function(req, res){

var index = '';
for (var i = 0; i < 1024 * 1024; i++) {
index += 'x';
}

res.send(index);
});

The above works fine with no issues. However, if I serve a text file
(.js) of that size it hangs at about 64k still. I know about the static
file serving in Passenger, so I ensured that the files were actually
being served through connect/express and not directly (directly works
fine). I actually had been using log level 3 in passenger, and the
request simply stalls -- it looks like a buffer dries up, but I really
have no idea.

I'm sending the log as an attachment in a separate email directly to
you, I hope that's okay.

-Garret

[Steven Bull]

Hey Hongli,

As best as I can tell, it seems like passenger is still requiring passenger_node.js to exist in order for it to use app.js. When I delete passenger_node.js, I get a "403 Forbidden" response (on the root of my host, 404 Not Found on other paths). But if I "touch passenger_node.js", it runs app.js again (passenger_node.js is just an empty file).

I'm running 4.0.23 now (from apt repo) on Ubuntu 12.04, with Nginx 1.4.2.

[Hongli Lai]

On Tue, Nov 5, 2013 at 10:33 AM, Steven Bull <steve...@gmail.com> wrote:
> Hey Hongli,
>
> As best as I can tell, it seems like passenger is still requiring
> passenger_node.js to exist in order for it to use app.js. When I delete
> passenger_node.js, I get a "403 Forbidden" response (on the root of my host,
> 404 Not Found on other paths). But if I "touch passenger_node.js", it runs
> app.js again (passenger_node.js is just an empty file).
>
> I'm running 4.0.23 now (from apt repo) on Ubuntu 12.04, with Nginx 1.4.2.

Hi Steven. You need to upgrade your packages. The Ubuntu packages for
Phusion Passenger 4.0.23 are coupled with Nginx 1.4.3.

[Steven Bull]

:(

I believe I installed Nginx 1.4.2 from your apt repo, and all I have available now from that repo is 1.4.2-2~precise1 (aptitude show nginx). Do you recommend another way to get Nginx 1.4.3 via apt? Sorry if I'm being annoying and this is off topic for you (but I thought I read that your repo would have the latest Nginx releases also).

[Hongli Lai]

That's odd. Did you run apt-get update first? I just tested the
Phusion 'precise' repo and it gives me nginx-full
1:1.4.3-2.4.0.23~precise1 just fine.

> --
> You received this message because you are subscribed to the Google Groups
> "Phusion Passenger Discussions" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to phusion-passen...@googlegroups.com.
> To post to this group, send email to phusion-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/phusion-passenger.
> For more options, visit https://groups.google.com/groups/opt_out.

[Steven Bull]

Nevermind, I'm an idiot.

After upgrading to nginx 1.4.3 (from your apt repo), it appears to work without the passenger_node.js file's existence.

I'm marking this topic as completed :)

[Hongli Lai]

An `apt-get install nginx-full passenger` should be all that's needed.

Let's go over your configuration. Could you post the contents of your
/etc/apt/sources.list.d/passenger.list?

On Tue, Nov 5, 2013 at 7:55 PM, Steven Bull <steve...@gmail.com> wrote:
> Yeah, I had run apt-get update before trying. At present, my suspicion is
> that the version I'm getting from "sudo aptitude show nginx-full" (Version:
> 1:1.4.2-2~precise1) is being reported from the local cache, as it's already
> installed.
>
> I just tried adding the passenger repo on another ubuntu 12.04.3 server,
> where I had the default nginx installed (Version: 1.1.19-1ubuntu0.2) and
> didn't have passenger installed. After adding the passenger repo, "sudo
> aptitude show passenger" gives me Version: 1:4.0.23-1~precise1, but "sudo
> aptitude show nginx-full" still gives me Version: 1.1.19-1ubuntu0.2. So it
> appears that the nginx distribution from the passenger repo isn't being
> seen. :(
>
> I'm not very fluent in apt commands to figure out what's exactly happening,
> but I'm happy to try to learn and figure it out - are you able to point me
> to a particularly incantation of "aptitude foo ..." or "apt-get bar ..." (or
> "dpkg baz ..."?) that would be useful for debugging this?

[Hongli Lai]

Hey Steven. I just want to let you know that making http.Server
"magically" work without code changes, turns out to be a great
decision. Quite some people have been experimenting with Phusion
Passenger and Node.js lately. The magic hasn't failed so far. Some
people are very skeptical about Phusion Passenger's usefulness. Were
it not for the magic, I'm sure a lot of people wouldn't even have
bothered trying. Thanks for pushing your idea!