lindleycb / meteor-stale-session

Stale session and session timeout handling for meteorjs
MIT License
40 stars 20 forks source link

Added .getTime() to insure a millisecond comparison to the heartbeat #2

Closed Meloncon closed 10 years ago

Meloncon commented 10 years ago

My heartbeat, by default, was being stored in milliseconds. The overdueTimestamp seems to be in seconds. Comparison failed every time for me. Adding ".getTime()" insures that the overdueTimeout variable returns a value in milliseconds to the Mongo update routine. This should compare the heartbeat-in-milliseconds to overdueTimeout-in-milliseconds, instead of comparing milliseconds to seconds.

lindleycb commented 10 years ago

Hi Matt,

Thanks for taking an interest. I'm surprised that you have a requirement for a sub-second heartbeat - what are you working on that needs that kind of granularity!

Not sure why the comparison would be failing for you as everything involved (user.heartbeat, now, overdueTimestamp) is a Javascript Date type. MongoDB should be capable of doing Date<Date comparisons all the way down to the millisecond so I'm a little confused as to why it shouldn't work as is.

I've just dropped in a couple of console.logs to check and got the following:

screen shot 2013-11-29 at 13 52 24

As you say showing milliseconds

screen shot 2013-11-29 at 13 52 39

As you say just to the second - but is that just because that's the level that Date.toString is showing?

Can you be sure that the millisecond thing is what was causing a problem for you as I'd rather stick with keeping everything as a Date unless there's definitely a problem with the comparison.

Chris

P.S. Just realised that the project that I developed this for, I use date.js too which extends Date's prototype. Not sure if that might be making a difference one way or the other for me?

Meloncon commented 10 years ago

Hello Chris,

Thanks for providing this package! It's great to see a community building up around Meteor.

When I run 'meteor mongo' and do a 'db.users.findOne()' I get the following:

heartbeat screenshot

It seems to not store the heartbeat time as an ISODate in Mongo for me. Not sure why. If it was stored as a date that would definitely fix the problem. Mine seems to be stored as an integer containing the timestamp in milliseconds.

I think it's using Date.now() in the Meteor.method "heartbeat", to store the timestamp. According to the MDN, "The Date.now() method returns the number of milliseconds elapsed since 1 January 1970 00:00:00 UTC."

Here's the link: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now

That's what spurred me to do a .getTime() on the mongo comparison later, to convert the overdueTimestamp to milliseconds and compare it against the heartbeat. I agree with you though, I would rather use Date throughout and not do a conversion, as well.

Perhaps it is something to do with Date.js extending the Date object?

--Matt

lindleycb commented 10 years ago

Matt,

Not sure why I used Date.now() in the first place rather than just plain old "new Date()". I've just changed this and re-tested and it's putting it in the database as a Date for me. I think the problem must have been down to the difference in Date.now() with and without datejs. Can you upgrade to v1.0.3 and see if this fixes the problem for you?

Chris

Meloncon commented 10 years ago

Great! v1.0.3 did fix the problem for me. Should be good for others who aren't using Date.js, as well. Thanks again for this package, it has been quite nice not having to build it all myself.

lindleycb commented 10 years ago

No problem! I've been a major user of open source software since the FSF days and this is my first ever, very long overdue and very minor contribution. Glad it's been of use to more than just me and sorry that I put out the first version without realising the dependency on date.js!