Wizcorp / node-graylog2

Graylog2 client library for Node.js
Other
58 stars 36 forks source link

possible typo in graylog2.js line 118 #25

Open declansto opened 7 years ago

declansto commented 7 years ago

line 112 - 121

var payload, fileinfo, that = this, field = '', message = { version : '1.0', 118 -->> timestamp : (timestamp || new Date()).getTime() / 1000, <<-- host : this.hostname, facility : this.facility, level : level };

ronkorving commented 7 years ago

I assume you mean to say that timestamp is a number, and this code runs .getTime() on it? I think the original author of that line probably intended for that argument to be a Date object. But I think you're right, it's more natural to support a number than it is a Date object, although both could easily be supported.

Can you confirm if I understood you correctly?

declansto commented 7 years ago

My comment was that I needed to remove the '(' in timestamp : ( timestamp || .. before the code would run. This looks like a typo in the code.

ronkorving commented 7 years ago

The code does not have a brace too many. It wouldn't run if that were the case, and this module does run.

Am I misunderstanding? Can you double check?

declansto commented 7 years ago

/home/wizrd/ic-schedulergraphrunner/node_modules/graylog2/graylog.js:118 timestamp : (timestamp || new Date()).getTime() / 1000, ^

TypeError: (timestamp || (intermediate value)).getTime is not a function at graylog.log [as _log] (/home/wizrd/ic-schedulergraphrunner/node_modules/graylog2/graylog.js:118:52) at graylog.error (/home/wizrd/ic-schedulergraphrunner/node_modules/graylog2/graylog.js:88:17) at processNewGraphs (/home/wizrd/ic-schedulergraphrunner/furoMCR.js:296:32) at Request._callback (/home/wizrd/ic-schedulergraphrunner/furoMCR.js:186:17) at Request.self.callback (/home/wizrd/ic-schedulergraphrunner/node_modules/request/request.js:186:22) at emitTwo (events.js:106:13) at Request.emit (events.js:194:7) at Request. (/home/wizrd/ic-schedulergraphrunner/node_modules/request/request.js:1081:10) at emitOne (events.js:96:13) at Request.emit (events.js:191:7)

ronkorving commented 7 years ago

I think the idea is that you pass a Date instance, not a number.