carlos8f / buffet

Performance-oriented static file server
http://carlos8f.github.com/node-buffet
190 stars 12 forks source link

Bug? First Page Load Causes 404? #9

Closed jprichardson closed 11 years ago

jprichardson commented 11 years ago

Hey Carlos,

I believe that I found a new bug in 0.5.0. This bug doesn't exist in 0.4.x. Unfortunately, this bug is affecting automated tests of any of my programs that use buffet.

In short, when a Node.js process is first started with a buffet (0.5) server, on first HTTP request it responds with a 404 error really quickly and then a new page with 200.

Here is how you can reproduce it:

mkdir /tmp/buffettest
cd /tmp/buffettest
touch server.js
touch index.html
npm install buffet@latest

Put the following in server.js:

var http = require('http')
  , buffet = require('buffet')

var server = http.createServer()
  , fileServer = buffet({root: './'})

server.on('request', fileServer)
server.on('request', fileServer.notFound);
server.listen(2222)

Put something in index.html:

hello

Now, open up Chrome and then open up the 'Developer Tools' pane at the bottom. Go to 'Network' tab. Enter in your address to the buffet server: http://localhost:2222. You'll get a 404. Refresh the page, and you'll get the expected HTML file.

Like I said, this behavior does not exist with buffet 0.4.x.

I'm using Mac OS X 10.8.2. and Node v0.8.16.

Thanks.

carlos8f commented 11 years ago

Thanks for this bug report. I can reproduce it and it is related to the new buffet logic being abstracted into a new module, dish.

There are two things that together cause this:

  1. The server.on('request') examples in the README.md are not quite correct, for the reason that node will not execute the handlers as middleware, but rather trigger them in series without waiting for the next() callback. It worked though in 0.4.x, because buffet used to be able to end the response synchronously (async setup is done at the point of creating the middleware handler function) and therefore the 404 handler can't end the response because it's already sent.
  2. node-dish does its async setup (i.e. gzipping the data) on request (lazy-load) instead of when the callback function is created. For node-dish, I think this is ideal, so I'm not going to change that. But it does however break buffet's ability to use (albeit hackishly) middleware as vanilla request handlers.

It would be possible to make your example work by using a synchronous gzip library like compress-buffer in node-dish, but I am trying to get away from depending on synchronous stuff for syntax sugar since it blocks the event loop. I'd rather that you used a proper middleware runner like connect or middler, or something like this:

server.on('request', function (req, res) {
  fileServer(req, res, function () {
    fileServer.notFound(req, res);
  });
});

I'll fix the README example with the above, also.

Cheers,

Carlos

jprichardson commented 11 years ago

Awesome, thanks... works great.