wandenberg / nginx-push-stream-module

A pure stream http push technology for your Nginx setup. Comet made easy and really scalable.
Other
2.21k stars 295 forks source link

lp - fix eventid #270

Open kamil-michalak opened 6 years ago

kamil-michalak commented 6 years ago

Long polling - missing eventid parameter

wandenberg commented 6 years ago

Hi @kilgor the eventid was added to work with EventSource mode, where while connecting the client "say" what was the last message it received and (ideally) do not disconnect. Of course, it can be used with the other modes as well, but what will happen if not all messages have an event id?! My experience say that the client will receive "duplicate" messages. For example,

Also, can you ensure the current tests for javascript are running and add some more to your changes?

kamil-michalak commented 6 years ago

Hi @wandenberg EventId is great if we have more than one publishing server. This is the way to enumarate messages by own publishing system. This paramater work with websocket and eventsource and stream module. I trying to implement own "numbering" messages by this way. I know if i use the same event id i can duplicate messages. I tested that i can remove time and etag paramaters and replace them by eventid. But currently js implementation loose this value.

wandenberg commented 6 years ago

Not saying to remove time and etag, just to make them compatible. Something like, when the last received message doesn't have the eventid, erase the value in the pushstream instance. This way, in the next connection only time and tag will be sent to the server and will avoid sending duplicated messages to the client. I am guessing that in your use case all messages have an eventid, but this isn't true to everyone, at least not required. What do you think?