bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
798 stars 186 forks source link

responseWhitelist.push('body') crashes #52

Closed arnaugm closed 9 years ago

arnaugm commented 9 years ago

I'm trying to log the body of the response and apparently the lib crashes when trying to get it from the headers.

Simple example app:

var express = require('express');
var expressWinston = require('express-winston');
var winston = require('winston');
var app = express();

expressWinston.responseWhitelist.push('body');
app.use(expressWinston.logger({
    transports: [
        new winston.transports.Console({})
    ]
}));

app.get('/', function (req, res) {
    res.send('Hello World!')
});

var server = app.listen(3000, function () {

    var host = server.address().address;
    var port = server.address().port;

    console.log('Example app listening at http://%s:%s', host, port)

});

Accessing the defined route results in a crash

> node app.js
Example app listening at http://0.0.0.0:3000
TypeError: Cannot call method 'indexOf' of undefined
    at ServerResponse.res.end (/home/arnau/code/myapp/node_modules/express-winston/index.js:201:64)
    at ServerResponse.res.send (/home/arnau/code/myapp/node_modules/express/lib/response.js:154:8)
    at app.listen.host (/home/arnau/code/myapp/app.js:14:9)
    at callbacks (/home/arnau/code/myapp/node_modules/express/lib/router/index.js:164:37)
    at param (/home/arnau/code/myapp/node_modules/express/lib/router/index.js:138:11)
    at pass (/home/arnau/code/myapp/node_modules/express/lib/router/index.js:145:5)
    at Router._dispatch (/home/arnau/code/myapp/node_modules/express/lib/router/index.js:173:5)
    at Object.router (/home/arnau/code/myapp/node_modules/express/lib/router/index.js:33:10)
    at next (/home/arnau/code/myapp/node_modules/express/node_modules/connect/lib/proto.js:193:15)
    at Object.handle (/home/arnau/code/myapp/node_modules/express-winston/index.js:231:9)
floatingLomas commented 9 years ago

What version of winston-express are you using?

arnaugm commented 9 years ago

I just created this example from scratch to check that the issue was not fixed yet, so I used the 0.2.9. In another project I was using 0.2.8 and I had the same problem.

floatingLomas commented 9 years ago

Yup. I believe that my PR will fix it, so it's up to @bithavoc to pull the trigger on the merge.

bithavoc commented 9 years ago

Publishes to NPM as 0.2.11

arnaugm commented 9 years ago

0.2.11 corrects the crash, but the body field is empty. In the previous example, the body should be 'Hello World!'

info: HTTP GET / url=/, host=localhost:3000, connection=keep-alive, cache-control=max-age=0, accept=text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,/;q=0.8, user-agent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/39.0.2171.65 Chrome/39.0.2171.65 Safari/537.36, accept-encoding=gzip, deflate, sdch, accept-language=en-US,en;q=0.8,ca;q=0.6,es;q=0.4, if-none-match="472456355", method=GET, httpVersion=1.1, originalUrl=/, , body=undefined, statusCode=304, body=, responseTime=2

bithavoc commented 9 years ago

can't see the content type of the request @arnaugm

see this: https://github.com/itagenten/express-winston/commit/c2f2ba944269bb2a9d3ba1e8a18916ee0c6fe038

arnaugm commented 9 years ago

Sorry @bithavoc I don't understand your question.

bithavoc commented 9 years ago

@arnaugm I don't see the content type of the request, is it there in the headers?

arnaugm commented 9 years ago

Not sure what you mean. My point is that with the example above I think it should work, Am I wrong? If the content type is not specified, the default one should be taken into account and in any case return the content of the body.

floatingLomas commented 9 years ago

This problem should be fixed in 2.11 (newest version) - the behaviour you describe is what now happens.

arnaugm commented 9 years ago

Seems to work now, great!

floatingLomas commented 9 years ago

Cool. :)