Skip to content

batch_presence.md's restricted-key test closes the connection that owns the presence member it then asserts on #548

Description

@owenpearson

Summary

rest/integration/RSC24/restricted-key-channel-failure-1 enters a presence member, closes the
realtime connection that entered it, and then asserts that a REST batchPresence finds it. A
presence member belongs to the connection that entered it, so the close is what removes the thing
the test is about. Measured against the sandbox, the allowed channel's result comes back carrying
no presence key at all, every time.

The other two tests in the same file get this right and say so in a comment. The fix is to move
AWAIT realtime.close() out of the setup and into cleanup, matching them.

Line references are against d9a04ca, which is still main; paths are relative to uts/.


1. rest/integration/batch_presence.md:196 — AWAIT realtime.close() sits in the setup, before the read

The setup (:172-197):

realtime.connect()
AWAIT_STATE realtime.connection.state == ConnectionState.connected

ch_allowed = realtime.channels.get(allowed_channel)
AWAIT ch_allowed.attach()
AWAIT ch_allowed.presence.enterClient("member-1", data: "hello")

ch_denied = realtime.channels.get(denied_channel)
AWAIT ch_denied.attach()
AWAIT ch_denied.presence.enterClient("member-2", data: "world")

AWAIT realtime.close()          # <- :196

and the assertions (:213-230):

ASSERT result.successCount == 1
ASSERT result.failureCount == 1
...
ASSERT success IS BatchPresenceSuccessResult
ASSERT success.presence.length == 1
ASSERT success.presence[0].clientId == "member-1"

The cleanup section (:233-236) then explains the close away — "No cleanup needed — the Realtime
client was already closed during setup" — so the placement is deliberate rather than a stray line.

Measured against the sandbox. Three runs of the setup exactly as written, each querying
GET /presence?channels=channel6,denied-… with the restricted key before the close and then at
+0 s, +1 s and +3 s after it:

run 1 BEFORE close -> 200  successCount=1 failureCount=1 | channel6: presence=['member-1'] | denied-batch--fWoESUe: error 40160/401
run 1 AFTER close (+0.0s) -> 200  successCount=1 failureCount=1 | channel6: NO 'presence' KEY (keys=['channel']) | denied-batch--fWoESUe: error 40160/401
run 1 AFTER close (+1.0s) -> 200  successCount=1 failureCount=1 | channel6: NO 'presence' KEY (keys=['channel']) | denied-batch--fWoESUe: error 40160/401
run 1 AFTER close (+3.0s) -> 200  successCount=1 failureCount=1 | channel6: NO 'presence' KEY (keys=['channel']) | denied-batch--fWoESUe: error 40160/401

run 2 BEFORE close -> 200  successCount=1 failureCount=1 | channel6: presence=['member-1'] | denied-batch-PkWC5f4x: error 40160/401
run 2 AFTER close (+0.0s) -> 200  successCount=1 failureCount=1 | channel6: NO 'presence' KEY (keys=['channel']) | denied-batch-PkWC5f4x: error 40160/401
...
run 3 BEFORE close -> 200  successCount=1 failureCount=1 | channel6: presence=['member-1'] | denied-batch-l9YbPpKs: error 40160/401
run 3 AFTER close (+0.0s) -> 200  successCount=1 failureCount=1 | channel6: NO 'presence' KEY (keys=['channel']) | denied-batch-l9YbPpKs: error 40160/401

The full body after the close:

{
  "successCount": 1,
  "failureCount": 1,
  "results": [
    { "channel": "channel6" },
    { "channel": "denied-batch-GEDLkiiZ",
      "error": { "code": 40160, "statusCode": 401,
                 "message": "Unauthorized to request presence for this channel",
                 "href": "https://help.ably.io/error/40160" } }
  ]
}

and with the connection left open, which is the only change:

{
  "successCount": 1,
  "failureCount": 1,
  "results": [
    { "channel": "channel6",
      "presence": [ { "action": 1, "clientId": "member-1", "connectionId": "CYQXv3vF6m",
                      "data": "hello", "id": "CYQXv3vF6m:0:0", "timestamp": 1790286730935 } ] },
    { "channel": "denied-batch-jbarlXfx",
      "error": { "code": 40160, "statusCode": 401, ... } }
  ]
}

So successCount, failureCount and the whole BGF2 half of the test are unaffected by the close —
only the two assertions that read success.presence fail, and they fail every time, not
intermittently. This is not a race that a wait would fix.

2. The file's own other two tests already have the right shape

rest/integration/RSC24/batch-presence-multiple-channels-0 puts the close in cleanup (:154-157)
and marks the test steps "keep realtime open so presence persists" (:118).
rest/integration/RSC24/empty-channel-presence-2 is explicit (:266-268):

# NOTE: Keep realtime open during the REST query so the presence member
# persists on the server. Closing realtime before the query would cause
# the member to leave.

The restricted-key test is the odd one out in its own file.


Suggested fix

Delete :196 and give the test the cleanup section its two siblings have. Replace :196 with
nothing and :233-236 with:

### Cleanup
AWAIT realtime.close()

and add the same note to the test steps that :118 already carries. :201's comment is stale in
its own right — it names a channel the setup does not use (the setup fixes allowed_channel = "channel6" at :175, and "batch-allowed" appears nowhere in the file) — so both lines are worth
replacing together:

# Query with the restricted key, which has capability only for "channel6".
# Keep realtime open: the presence members belong to that connection, and
# closing it before the query removes them.
restricted_rest = Rest(options: ClientOptions(
  key: restricted_key,
  endpoint: "nonprod:sandbox",
  useBinaryProtocol: PROTOCOL == "msgpack"
))

Nothing in the assertions needs to change.


Checked and not filed: the same shape in presence.md survives

rest/integration/RSP4b2/history-direction-forwards-0 (presence.md:313-360) looks like the same
fault — it closes the connection at :338 and then asserts on the newest item of presence history
at :359:

AWAIT realtime_channel.presence.enter(data: "first")
AWAIT realtime_channel.presence.update(data: "second")
AWAIT realtime_channel.presence.update(data: "third")
AWAIT realtime.close()
...
history_backwards = AWAIT rest_channel.presence.history(direction: "backwards")
ASSERT history_backwards.items[0].data == "third"

The close does add a fourth event: a LEAVE, which becomes the newest item and is what items[0]
then reads. But the server's synthesized LEAVE inherits the member's last data, so the
assertion holds anyway. Four consecutive runs:

run 1: 4 items; newest action=3 data='third'; ASSERT items[0].data == "third" -> PASS
run 2: 4 items; newest action=3 data='third'; ASSERT items[0].data == "third" -> PASS
run 3: 4 items; newest action=3 data='third'; ASSERT items[0].data == "third" -> PASS
run 4: 4 items; newest action=3 data='third'; ASSERT items[0].data == "third" -> PASS

(the full backwards page, with the close: LEAVE "third", UPDATE "third", UPDATE "second",
ENTER "first"; without it: UPDATE "third", UPDATE "second", ENTER "first".)

So RSP4b2 is not broken, and we are not asking for it to change. It is only worth noting that the
test's items[0] is a LEAVE rather than the update it reads as, which is a coincidence of the
server preserving the data rather than something the test establishes. If the RSL2b/RSP4b files are
being touched anyway, asserting items[0].action alongside the data would pin what is intended.

Found while deriving uts/rest/integration for ably-python.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions