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

fixes for disonnect and decode #607

Merged
merged 3 commits into from Nov 9, 2022

Conversation

yadue
Copy link
Contributor

@yadue yadue commented Nov 9, 2022

fix: fix for extremely rare json decode issue
fix: prevent breaking next transport timeout if previous transport fa…

this PR contains 2 bugfixes we implemented in our sockjs fork. they never got merged as previous owner declined them since he couldn't reproduce the issue.

Decode issue occurs in rare occasions with spring boot sockjs integration. Sometimes sockjs was not able to parse message correctly.

Second issue is little bit easier to reproduce. If we set transports to
[ 'eventsource', 'iframe', 'htmlfile', 'xhr-streaming',]
and them block with ublock eventsource, xhr, xhr_streaming them xhr when failing does not clear its timeout and when next transport kicks in then it's after ~ 20ms being cleared by previous transport timeout. This clears the timeout if the transport failed before its timeout kicked in. Currently if transport fails before its timeout then the next transport is being initialised with new timeout but the old timeout is still in a queue.

Take it or drop it.

@yadue yadue marked this pull request as ready for review November 9, 2022 06:29
Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please add some test to verify the changes?

@yadue
Copy link
Contributor Author

yadue commented Nov 9, 2022

@auvipy added units, old pr here for time being #490

@auvipy auvipy merged commit 52acd8e into sockjs:main Nov 9, 2022
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.

None yet

2 participants