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

RealtimePresence#leave is broken when passing a PresenceMessage instance #1465

Closed
lawrence-forooghian opened this issue Oct 20, 2023 · 1 comment · Fixed by #1466
Closed
Assignees
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Oct 20, 2023

RealtimePresence’s enter*, update* and leave* methods all advertise themselves as accepting a PresenceMessage instance instead of just a data object. This is the only way that ably-js supports passing message extras to these methods.

However, leave does not handle a PresenceMessage argument properly. Update the presenceMessageExtras test case in test/realtime/presence.test.js and you'll see it fail:

Test case
/*
 * Attach to channel, enter presence channel with extras and check received
 * PresenceMessage has extras. Then do the same for leaving presence.
 */
it('presenceMessageExtras', function (done) {
  var clientRealtime = helper.AblyRealtime({ clientId: testClientId, tokenDetails: authToken });
  var channelName = 'presenceEnterWithExtras';
  var clientChannel = clientRealtime.channels.get(channelName);
  var presence = clientChannel.presence;

  async.series(
    [
      function (cb) {
        clientChannel.attach(cb);
      },
      // Test entering with extras
      function (cb) {
        presence.subscribe(function presenceListener(presenceMessage) {
          presence.unsubscribe(presenceListener);
          try {
            expect(presenceMessage.extras).to.deep.equal(
              { headers: { key: 'value' } },
              'extras should have headers "key=value"'
            );
          } catch (err) {
            cb(err);
            return;
          }
          cb();
        });
        presence.enter(
          PresenceMessage.fromValues({
            extras: { headers: { key: 'value' } },
          })
        );
      },
      // Test leaving with extras
      function (cb) {
        presence.subscribe(function presenceListener(presenceMessage) {
          presence.unsubscribe(presenceListener);
          try {
            expect(presenceMessage.extras).to.deep.equal(
              { headers: { otherKey: 'otherValue' } },
              'extras should have headers "otherKey=otherValue"'
            );
          } catch (err) {
            cb(err);
            return;
          }
          cb();
        });
        presence.leave(
          PresenceMessage.fromValues({
            extras: { headers: { otherKey: 'otherValue' } },
          })
        );
      },
    ],
    function (err) {
      if (err) {
        closeAndFinish(done, clientRealtime, err);
        return;
      }
      closeAndFinish(done, clientRealtime);
    }
  );

  monitorConnection(done, clientRealtime);
});

It fails because the received presence message looks like this:

PresenceMessage {
  id: 'K92Dg-VUnC:1:0',
  clientId: 'testclient',
  connectionId: 'K92Dg-VUnC',
  timestamp: 1697827112736,
  encoding: null,
  data: { action: -1, extras: { headers: [Object] } },
  action: 'leave'
}

that is, the extras end up incorrectly being nested inside the message’s data.

@lawrence-forooghian lawrence-forooghian added the bug Something isn't working. It's clear that this does need to be fixed. label Oct 20, 2023
@lawrence-forooghian lawrence-forooghian self-assigned this Oct 20, 2023
@sync-by-unito
Copy link

sync-by-unito bot commented Oct 20, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3910

lawrence-forooghian added a commit that referenced this issue Oct 20, 2023
RealtimePresence’s `enter*`, `update*` and `leave*` methods all
advertise themselves as accepting a PresenceMessage instance instead of
just a data object. This is the only way that ably-js supports passing
message extras to these methods.

However, `leave` does not handle a PresenceMessage instance properly; it
ends up nesting the PresenceMessage’s top-level fields inside the
payload’s `data` property.

To fix this, we change the argument handling to match that used by
`enter`.

Resolves #1465.
lawrence-forooghian added a commit that referenced this issue Oct 20, 2023
RealtimePresence’s `enter*`, `update*` and `leave*` methods all
advertise themselves as accepting a PresenceMessage instance instead of
just a data object. This is the only way that ably-js supports passing
message extras to these methods.

However, `leave` does not handle a PresenceMessage argument properly; it
ends up nesting the PresenceMessage’s top-level fields inside the
payload’s `data` property.

To fix this, we change the argument handling to match that used by
`enter`.

Resolves #1465.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

Successfully merging a pull request may close this issue.

1 participant