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

Duplicate messages emitted after error in onmessage #563

Closed
jcheng5 opened this issue Sep 8, 2021 · 6 comments · May be fixed by #564
Closed

Duplicate messages emitted after error in onmessage #563

jcheng5 opened this issue Sep 8, 2021 · 6 comments · May be fixed by #564

Comments

@jcheng5
Copy link

jcheng5 commented Sep 8, 2021

Using xhr-streaming, xhr-polling, or jsonp-polling, if an error is raised in the socket.onmessage event handler, then the next time a message is received SockJS will replay the message that caused the error.

For example, message A is received and the client onmessage event handler throws an uncaught error; later, message B is received, and onmessage is called again with message A.

Repro repo: https://github.com/jcheng5/sockjs-error-test

The problem appears to be here:

for (var idx = -1; ; this.bufferPosition += idx + 1) {
var buf = text.slice(this.bufferPosition);
idx = buf.indexOf('\n');
if (idx === -1) {
break;
}
var msg = buf.slice(0, idx);
if (msg) {
debug('message', msg);
this.emit('message', msg);
}
}
};

When this.emit('message', msg); throws an exception, this.bufferPosition doesn't get updated (third clause of the for-loop). If you update this.bufferPosition before emitting the message, the problem goes away.

@aronatkins
Copy link

This appears to be the 0.3.4 equivalent code, which updates the buffer position ahead of sending an event and does not suffer from duplicate messages after error:

that.xo.onchunk = function(status, text) {
if (status !== 200) return;
while (1) {
var buf = text.slice(buf_pos);
var p = buf.indexOf('\n');
if (p === -1) break;
buf_pos += p+1;
var msg = buf.slice(0, p);
that.dispatchEvent(new SimpleEvent('message', {data: msg}));
}
};

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@aronatkins
Copy link

Discussion has been happening in #564; false no-activity.

@github-actions
Copy link

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

@aronatkins
Copy link

False no-activity. Discussion is in #564; waiting for feedback from @brycekahle.

@aronatkins
Copy link

Ping to keep open; #564 is still waiting for feedback from @brycekahle.

whimsicallyson pushed a commit to posit-dev/sockjs-client that referenced this issue May 8, 2024
whimsicallyson pushed a commit to posit-dev/sockjs-client that referenced this issue May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants