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

Detach fails with timeout #1932

Open
maratal opened this issue Jun 9, 2024 · 4 comments
Open

Detach fails with timeout #1932

maratal opened this issue Jun 9, 2024 · 4 comments
Assignees
Labels
investigate Requires further investigation to decide the most appropriate label(s).

Comments

@maratal
Copy link
Collaborator

maratal commented Jun 9, 2024

"test__063__Channel__detach__happens_when_channel_is_ATTACHED_if_connection_is_currently__DISCONNECTED" fails sometimes with "Attached is not equal to Attaching", since it asserts channel to be in the Attaching state before the call to detach. But it claims in the name that the state should be Attached. If you wait for the channel to attach first, then this test will fail, because RTL5h and RTL4i implementation clash - once disconnected, call to detach will fall into a queue leading the channel to be detached once CONNECTED (sending queued messages), but at the same time the realtime explicitly will call attach once CONNECTED leading two messages (Attach and Detach) sent to the server leading the detach call fail with timeout.

Also happens in "test__105__Connection__Transport_disconnected_side_effects__should_resent_the_DETACH_message_if_there_are_any_pending_channels" which is why it was skipped I guess.

┆Issue is synchronized with this Jira Task by Unito

@maratal maratal self-assigned this Jun 9, 2024
@maratal
Copy link
Collaborator Author

maratal commented Jun 16, 2024

Turned out detach call fails with timeout if called during brief disconnection regardless ATTACH message sent after it. It only works if the channel successfully re-attaches first.

@maratal maratal added the investigate Requires further investigation to decide the most appropriate label(s). label Jun 16, 2024
@maratal
Copy link
Collaborator Author

maratal commented Jul 14, 2024

This is with logging on websocket level. After the message was sent nothing is received until timeout. Looks like realtime wants ATTACH message first.

Screenshot 2024-07-14 at 18 51 47

@owenpearson @lmars

@owenpearson
Copy link
Member

@maratal i just tested this quickly, it seems that if you create a realtime connection and just send a DETACH for a random channel without attaching first, realtime still responds with a DETACHED, so I don't think it's correct that realtime wants an ATTACH message first. i'm not sure why you're not getting a DETACHED message in that test though.

@maratal maratal changed the title Attach/detach clash violating RTL5h/RTL4i Detach fails with timeout Jul 21, 2024
@maratal
Copy link
Collaborator Author

maratal commented Jul 23, 2024

if you create a realtime connection and just send a DETACH for a random channel without attaching first, realtime still responds with a DETACHED

If I call detach without attaching first (channel is in the INITIALIZED state) the library will just do nothing and callback with success (as per RTL5a). The problem lays in a different setup - you first wait until the channel is ATTACHED and then abruptly disconnect the client. That's where the behavior is strange - after connection re-established the first DETACH message will be ignored. If you call detach shortly before disconnect, then upon connection re-established and you re-send the DETACH message, the realtime behaves as expected responding with the DETACHED.

Summary: if the channel is ATTACHED, the realtime ignores DETACH after the connection recovery. @owenpearson @ttypic @SimonWoolf

You can quickly check it in "test__105__Connection__Transport_disconnected_side_effects__should_resent_the_DETACH_message_if_there_are_any_pending_channels" test by unskipping it (I would comment defer { client.dispose(); client.close() } line to not confuse detaching upon channel disposal).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Requires further investigation to decide the most appropriate label(s).
Development

No branches or pull requests

2 participants