Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Removed 'end' event listener. #87

Merged
merged 1 commit into from

2 participants

@arjansingh

The example did not work out of the box. I traced the problem to the 'end' event listener attached to request.

I guess older versions of node.js required this but as of 0.10.0 (probably earlier), the createServer callback isn't called until request has already completed.

Therefore, there is no need for the 'end' event listener anymore.

Also, I removed some unnecessary parentheses.

@arjansingh arjansingh Removed 'end' event listener.
The example did not work out of the box. I traced the problem to the 'end' event listener attached to request.

I guess older versions of node.js required this but as of 0.10.0 (probably earlier), the `createServer` callback isn't called until `request` has already completed.

Therefore, there is no need for the 'end' event listener anymore.

Also, I removed some unnecessary parentheses.
eeab04a
@phstc
Collaborator

Hey @arjansingh, great patch, thank you.

Are the tests still passing for you?

For me they stop at should be listening.

~/workspace/node-static(master ✔) npm test

> node-static@0.6.5 test /Users/pablo/workspace/node-static
> vows --spec --isolate


  ♢ node-static

  once an http server is listening with a callback
    ✓ should be listening

My node is v0.10.0.

Cheers

@arjansingh

You're welcome!

Looks like it' working for me:

    Switched to branch 'patch-1'
    [node-static] $npm test

    > node-static@0.6.5 test /Users/asingh/Projects/node-static
    > vows --spec --isolate


      ♢ node-static

      once an http server is listening with a callback
        ✓ should be listening

One thing, I did note though--and this is the same for master and patch-1--is that if you leave the test running for a minute or so you get:

streaming a 404 page
      ✗ should respond with 404 
        TypeError: Cannot read property 'statusCode' of undefined 
        at Request.suite.addBatch.addBatch.streaming a 404 page.should respond with 404 (/Users/asingh/Projects/node-static/test/integration/node-static-test.js:49:30) 
        at runTest (/usr/local/lib/node_modules/vows/lib/vows.js:132:26) 
        at EventEmitter.<anonymous> (/usr/local/lib/node_modules/vows/lib/vows.js:85:17) 
        at EventEmitter.emit (events.js:117:20) 
        at EventEmitter.options.Emitter.emit (/usr/local/lib/node_modules/vows/lib/vows.js:237:24) 
        at that.emitter.ctx (/usr/local/lib/node_modules/vows/lib/vows/context.js:31:52) 
        at Request.env.callback [as _callback] (/usr/local/lib/node_modules/vows/lib/vows/context.js:46:29) 
        at self.callback (/Users/asingh/Projects/node-static/node_modules/request/index.js:142:22) 
        at Request.EventEmitter.emit (events.js:95:17) 
        at Request.options.Emitter.emit (/usr/local/lib/node_modules/vows/lib/vows.js:237:24) 
        at ClientRequest.self.clientErrorHandler (/Users/asingh/Projects/node-static/node_modules/request/index.js:246:10) 
        at ClientRequest.EventEmitter.emit (events.js:95:17) 
        at ClientRequest.options.Emitter.emit (/usr/local/lib/node_modules/vows/lib/vows.js:237:24) 
        at Socket.socketOnEnd [as onend] (http.js:1521:9) 
        at Socket.g (events.js:175:14) 
        at Socket.EventEmitter.emit (events.js:117:20) 
        at Socket.options.Emitter.emit (/usr/local/lib/node_modules/vows/lib/vows.js:237:24) 
        at _stream_readable.js:872:14 
        at process._tickCallback (node.js:415:13)
      ✗ should respond with the streamed content 
          » expected 'Custom 404 Stream.', 
      got  undefined (==) // node-static-test.js:53

That might be another issue.

@phstc phstc referenced this pull request
Closed

Does this work? #89

@phstc phstc closed this
@phstc phstc reopened this
@arjansingh

Just to be clear that streaming a 404 page error appears on the tests irregardless of whether or not my change was made. You guys might want to open another issue to look into that.

@phstc phstc merged commit ee1cee6 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 25, 2013
  1. @arjansingh

    Removed 'end' event listener.

    arjansingh authored
    The example did not work out of the box. I traced the problem to the 'end' event listener attached to request.
    
    I guess older versions of node.js required this but as of 0.10.0 (probably earlier), the `createServer` callback isn't called until `request` has already completed.
    
    Therefore, there is no need for the 'end' event listener anymore.
    
    Also, I removed some unnecessary parentheses.
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 14 deletions.
  1. +9 −14 examples/file-server.js
View
23 examples/file-server.js
@@ -3,22 +3,17 @@ var static = require('../lib/node-static');
//
// Create a node-static server to serve the current directory
//
-var file = new(static.Server)('.', { cache: 7200, headers: {'X-Hello':'World!'} });
+var file = new static.Server('.', { cache: 7200, headers: {'X-Hello':'World!'} });
require('http').createServer(function (request, response) {
- request.addListener('end', function () {
- //
- // Serve files!
- //
- file.serve(request, response, function (err, res) {
- if (err) { // An error as occured
- console.error("> Error serving " + request.url + " - " + err.message);
- response.writeHead(err.status, err.headers);
- response.end();
- } else { // The file was served successfully
- console.log("> " + request.url + " - " + res.message);
- }
- });
+ file.serve(request, response, function (err, res) {
+ if (err) { // An error as occured
+ console.error("> Error serving " + request.url + " - " + err.message);
+ response.writeHead(err.status, err.headers);
+ response.end();
+ } else { // The file was served successfully
+ console.log("> " + request.url + " - " + res.message);
+ }
});
}).listen(8080);
Something went wrong with that request. Please try again.