Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Check this.connection._httpMessage for null - #25670

Closed
shanksauce wants to merge 1 commit into
nodejs:masterfrom
pristineio:fix/check-for-connection-null
Closed

shanksauce wants to merge 1 commit into
nodejs:masterfrom
pristineio:fix/check-for-connection-null

Conversation

@shanksauce

Copy link
Copy Markdown

When using express-ws, I've noticed a TypeError is thrown in early terminating router middleware. Here is code to reproduce the error:

var app = require('express')();
var expressWs = require('express-ws')(app);

// Terminating middleware
app.use('/ws', function(req, res, next) {
  try {
    return res.status(500).end();
  } catch(ex) {
    console.log(ex.name, ex.message);
  }
});

app.ws('/ws', function(ws, req) { });

app.listen(8888);

Package versions:

express: 4.13.1
express-ws: 0.2.6

@Fishrock123

Copy link
Copy Markdown

Looks like a bug, bookmarking in case it does not get noticed. @Fishrock123

@thefourtheye

Copy link
Copy Markdown

FWIW, io.js 2.3.4 and node 0.12.7 doesn't have this problem. Still, we need to fix this. 👍

thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Jul 13, 2015
thefourtheye added a commit to nodejs/node that referenced this pull request Jul 22, 2015
Refer: nodejs/node-v0.x-archive#25670

PR-URL: #2172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
@jasnell

jasnell commented Aug 6, 2015

Copy link
Copy Markdown
Member

@Fishrock123 ... did you ever have a chance to look at this?

@jasnell jasnell added the http label Aug 6, 2015
@Fishrock123

Copy link
Copy Markdown

@jasnell No I haven't.

@thefourtheye

Copy link
Copy Markdown

@jasnell this fix landed in io.js already

@jasnell

jasnell commented Aug 7, 2015

Copy link
Copy Markdown
Member

@thefourtheye .. ok, I'll queue it up to land in v0.12 as well. thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants