Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options at "conversations" publishing is ignored + Date client side #2

Closed
markudevelop opened this issue May 9, 2015 · 39 comments
Closed

Comments

@markudevelop
Copy link

Hi,

this.subscribe('conversations', {limit:1}); is ignored, It works fine on the messagesFor Publication.

Also there is a bigger issue I think with the message dates. When I chat and send new messages (using newMessage) there is a wrong order for a moment, because the client-side time and server-time is different so it's get sorted in the wrong order until simple-schema updates it via server new Date(). So typing quickly with different users timezones/times etc is a problem.

I'm not really sure if there is an easy solution for the dates can you force new message to be on top/bottom of the {{#each}} loop? Or set new date on client to some specific value?

Or just use https://github.com/mizzao/meteor-timesync?

@copleykj
Copy link
Owner

copleykj commented May 9, 2015

Ah, yes.. I seems when I switched to tmeasday:publish-with-relations I missed the options for the conversations query.. I'll get a fix out for that ASAP.

As for the dates, that could be a very annoying UI bug. Originally I hadn't added the date on the client side, but later decided to as a fix for another issue I was having. I think that mizzao:timesync or ground:servertime would probably be the best fix for this.. I'll have a look at both their API's in more detail and publish a fix for this with the fix for the limit on the conversations publication.

@markudevelop
Copy link
Author

@copleykj Hey, I just checked timesync seems heavy, ground is more lightweight but it calls the server we need speed. I think a better option will be to keep the users time zone offset in the meteor users collection (and publish it) moment().zone() this sounds like a better way?

@copleykj
Copy link
Owner

copleykj commented May 9, 2015

@voidale I'm not sure I like that storing the timezone is the best solution, but I'm also thinking that just using a server time package won't be a proper fix either.. Maybe a combination of a server time package and storing the date in UTC would be a better solution.. I'm open to suggestions if anyone else has any.

As for the server time packages I'm leaning towards ground.. It looks as if we would only have to get the server time at startup and then calls to ServerTime.now() would be fast.

@copleykj
Copy link
Owner

I pushed a fix for the publication.. I should be able to get to the other issues tomorrow morning and I'll publish an updated package.

@copleykj
Copy link
Owner

@voidale I just published a new release which definitely fixes the publication issue. Give the date issue a thorough test verify that it fixes the issue at hand. I ended up creating a new lighter weight package to compensate for the server time difference.. I liked the ground:servertime package API but not the fact that it required the ground:store package which is unnecessary for this use case.

@markudevelop
Copy link
Author

@copleykj Hey Thanks for such a quick fix :) I just tried the ServerTime package it seems to return wrong date.

ServerTime.date() Mon May 11 2015 09:06:58 GMT+0200 (Central Europe Daylight Time) and my server console returns on "date" Mon May 11 03:14:53 EDT 2015 It's GMT-0400

Calling Meteor.call("getServerTime" on the clients seems to return wrong date, Not sure why. But checked the mongodb and the message saved date is GMT -0400.

@copleykj
Copy link
Owner

Sorry about that.. I had an absolutely ridiculous error in the ServerTime.now() calculation.. I've updated the server-time package and updated the version dependency in messaging..

@markudevelop
Copy link
Author

@copleykj I'm not sure if this problem unique to my setup. But the server side method gets the wrong date. It's working fine for you? Meteor.call("getServerTime", function(error,serverTimeStamp){console.log(error); console.log(serverTimeStamp)}) on the browser console returns wrong date.

@copleykj
Copy link
Owner

The getServerTime method just returns Date.now() so the serverTimeStamp should log as a millisecond timestamp.

@markudevelop
Copy link
Author

@copleykj yeah I converted it at http://www.epochconverter.com/ and it's GMT +2 which is wrong. Is it correct one for you?

@copleykj
Copy link
Owner

Works perfectly on my end, and I'm not sure how it could be any other way except if there is another package that is overriding the method and returning an improper value.

@markudevelop
Copy link
Author

@copleykj where do you test it locally? I just created a new empty meteor project + meteor add socialize:server-time and in the browser console ServerTime.date() it's wrong it's on a digitalocean ubuntu.

@copleykj
Copy link
Owner

So far I've tested locally and on meteor.com servers.

@markudevelop
Copy link
Author

@copleykj http://servertime.meteor.com/ ServerTime.date() wrong for me how about for you? Or am I missing something this should be the server time right not mine? :)

@copleykj
Copy link
Owner

Yes, when I run it, I get 1431361151978 and the Epoch Converter gives me back the following...

GMT: Mon, 11 May 2015 16:50:23 GMT
Your time zone: 5/11/2015, 12:50:23 PM GMT-4:00 DST which is proper for my timezone

@copleykj
Copy link
Owner

Running ServerTime.date(), I'm getting back Mon May 11 2015 12:54:42 GMT-0400 (EDT) which is proper

@copleykj
Copy link
Owner

Ok, seems that the issue doesn't exist for me because the time difference is so close.. I'll have to contemplate this and I'll see about fixing asap

@copleykj
Copy link
Owner

ok, after thinking about this a bit... I think what you may be perceiving as an issue is actually just the natural conversion of the unix epoch timestamp to your local time. Timestamps being GMT based are automatically converted to your local time when constructing a new Date(), so the same timestamp on a machine with it's timezone set to GMT +1:00 would construct a different local representation then a machine set to GMT -5:00 even though they essentially refer to the same exact time.

So this package should fix an improperly set server time if that is the issue that is causing your UI to be finicky. If this doesn't fix the UI problem then we may be looking at something different altogether.

@markudevelop
Copy link
Author

@copleykj I'm really not sure about this one it's definitely odd that's it gets converted to something if we store it as a static string should be fine?, That's the whole problem for the UI. My new chat messages appear on top first for a moment and then shuffled to the bottom (new ones should be at bottom). The problem is at the client side because after the collection gets back the new message from server it has different date (correct one) and goes back to the bottom. basically latency compensation is against me this time. Either I have to disable client side collection or get fixed time from server ignore any conversation on client side with users time.

@copleykj
Copy link
Owner

As a quick check, if you go to the browser console and type ServerTime._timeDifference what does it return?

@markudevelop
Copy link
Author

@copleykj ServerTime._timeDifference
14520

@copleykj
Copy link
Owner

ok, so yeah, a 14 second time difference could cause some issues, but I would think this should fix it.. Is your app on a server that I can access and try to determine the issue.. Unfortunately I don't have an easy way check this out since all the servers I have access to are within a couple hundred milliseconds of my local machines time.

@markudevelop
Copy link
Author

@copleykj my server is also the same timezone as yours, I think changing the PC time should work? I will try to change GMT-4:00 see if this fixes my issue

@copleykj
Copy link
Owner

Changing the timezone on your machine unfortunately won't fix the issue due
to the fact that all dates are GMT based which is universal the world
around.
On May 11, 2015 3:07 PM, "voidale" notifications@github.com wrote:

@copleykj https://github.com/copleykj my server is also the same
timezone as yours, I think changing the PC time should work? I will try to
change GMT-4:00 see if this fixes my issue


Reply to this email directly or view it on GitHub
#2 (comment)
.

@markudevelop
Copy link
Author

@copleykj I definitely lost on how those dates work, So I will get you a server on Europe this should cause you to see the problem?

@copleykj
Copy link
Owner

copleykj commented May 11, 2015 via email

@markudevelop
Copy link
Author

@copleykj I have created a droplet where you can debug this one: IP Address: 46.101.50.222
Username: root password: your nick name here :)

@copleykj
Copy link
Owner

So I didn't have time to do much more than deploy a skeleton app to and run stuff through the command line, adding a conversation and then adding messages to the conversation and watching them change to verify that they weren't coming back from the server with different values.. All seems to be working even with an almost 35 minute time difference manually set on the server.

@markudevelop
Copy link
Author

@copleykj It's coming from the server fine. The problem is when the message is inserted to the client side. SimpleSchema/Collection2 autovalue inserts the message to client side first after a "moment" it gets updated from the server. If you query it manually you will probably miss it. Well it's getting too much complicated so sorry for getting your time wasted, I will just publish fixed server time and add message by server time that will solve the problem.

@copleykj
Copy link
Owner

Right, so the way I tested was to watch the last message on a conversation using a Tracker.autorun and logging out the message object.

Tracker.autorun(function(){
    console.log(Meteor.conversations.findOne().lastMessage());
});

This will log the last message out to the console anytime the last message changes, including when the latency compensation causes the date to come back with a different value from the server after simple-schema causes it to change.

Then I call Meteor.conversations.findOne().sendMessage("Message Text") to add a new message to the conversation. When I do this it logs the new message object once to the console. If there were a discrepancy in the times that would cause a UI flicker then it would log twice, once for the insert on the client and once when the server sent the document with the new date that had been inserted server side.

@markudevelop
Copy link
Author

@copleykj I see, well when I get this one fixed I will let you know what was the cause maybe I'm doing something wrong (hopefully) Thanks a lot for your help :)

@copleykj
Copy link
Owner

@voidale ok, I've deployed an application with a bit of an interface to test with on the DO server you provided.. Its pretty basic and adding conversations and participants is still done through the console, but I've set up a conversation and sent a couple messages back and forth to test it out a bit and it seems to work as expected. Give it a workout and see if you can get it to give you the same issue you are having. You can log in with the email voidale@test.com password abcdefg or copleykj@gmail.com with the same password.

@markudevelop
Copy link
Author

@copleykj just tried and it work as expected, Just needed to hit the messages limit (10) first only then you can see the issue. So just to be sure you can try writing 2 messages quickly now and if it works for you. It's my fault and I'm sorry not really sure if server-time package is actually not needed because of my mistake?

@copleykj
Copy link
Owner

So this is working without the UI issue for you?

@copleykj
Copy link
Owner

As for server-time package being needed, I think that it's a good addition being that the messaging package does add the date on the client side first as a way to mitigate the same UI issue when sorting but not having any date at all until the server sends back the new date.

@markudevelop
Copy link
Author

@copleykj yeah it works fine :) no issues.

@copleykj
Copy link
Owner

Ok. I've created a new repo and pushed out the code I had deployed to your DO droplet. Maybe you can get an idea of how to fix your UI issues by looking at the code. UI code is located in the views folder and is separated into pages and partials.

@markudevelop
Copy link
Author

@copleykj that will be really helpful, Thanks!

@copleykj
Copy link
Owner

You've very welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants