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

lp - fix eventid #270

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

kamil-michalak
Copy link

Long polling - missing eventid parameter

@wandenberg
Copy link
Owner

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,

  • message 1 has an eventid A
  • message 2 does not have an eventid
  • the client disconnect after receive message 2
  • when the client connect again, with your proposal, it will receive the message 2 again, right?!

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

@kamil-michalak
Copy link
Author

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
Copy link
Owner

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?

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

Successfully merging this pull request may close these issues.

2 participants