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.

Sign up for GitHub

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

Jump to bottom

[COL-335] Fix bug where space.enter() sometimes hangs. #227

Merged
merged 7 commits into from
Oct 19, 2023
Merged

[COL-335] Fix bug where space.enter() sometimes hangs. #227

merged 7 commits into from
Oct 19, 2023

Commits on Oct 19, 2023

  1. Remove mocking of presence enter completion in space.enter() test 

    space.enter() doesn’t await the result of the presence enter operation
(whether it should or not is another matter), and the other tests for
space.enter() don’t mock its completion, so let’s be consistent with
them.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    72696f8 View commit details
    Browse the repository at this point in the history

  2. Format auto-mocks

    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    f827d5a View commit details
    Browse the repository at this point in the history

  3. Make each test context create new mock objects 

    This removes the possibility of one test case affecting another due to
shared mock objects.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    c66bd09 View commit details
    Browse the repository at this point in the history

  4. Remove unnecessary space.enter() from some tests 

    These calls presumably exist because the method that they are testing
requires that the client has entered the space. But calling
`space.enter()` in the context of these tests does nothing useful — the
underlying Realtime object is a mock, and the `presenceMap` objects in
the tests’ contexts have already been configured such that a member is
present for the space’s connection ID.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    ce44eb9 View commit details
    Browse the repository at this point in the history

  5. Remove usage of private presence API 

    Instead of using the private `subscriptions` property, implement a
`once`-like behaviour by passing `subscribe` a listener that removes
itself once triggered.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    1431c3c View commit details
    Browse the repository at this point in the history

  6. Check sender of ENTER presence message in space.enter() 

    It shouldn’t return as a result of somebody else entering the space.

I’m not sure whether this is the best way of predicating whether or not
to return — perhaps checking for some unique ID on the received ENTER
presence message would be better. But it seems like an OK low-thought
solution at least for the time being.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    6f513a1 View commit details
    Browse the repository at this point in the history

  7. Fix bug where space.enter() sometimes wouldn’t return 

    Calling presence.enter() does not necessarily result in the presence
object emitting an ENTER event. This could happen, for example, if the
channel does not become attached quickly enough, or if a transport
upgrade is happening at roughly the same time as the presence enter call
(note that the latter possibility means that we wouldn’t gain much by
trying to work around the former by making sure the channel becomes
attached before performing the presence enter). In both these cases, the
only visible side effect of the presence enter call will be a PRESENT
event emitted as a result of a presence SYNC.

So, we update space.enter such that it also will return if it receives a
PRESENT event for the current member.

Resolves COL-335.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 19, 2023
    Configuration menu
    Copy the full SHA
    803416d View commit details
    Browse the repository at this point in the history