Skip to content

connection_failures_test.md's RTN14a and RTN14g are the same provocation, so the revoked-key spec point has no coverage #554

Description

@owenpearson

Summary

realtime/integration/connection/connection_failures_test.md holds two tests, presented as
"invalid API key" (RTN14a) and "revoked key / deleted app" (RTN14g). Both fixtures name an
application that does not exist, and the sandbox answers both identically: 40101 / 401,
unable to handle request; no application id found in request. Both sets of assertions admit that
code, so both pass. The two sections differ in what they assert about one shared server response,
not in the response they provoke.

Neither test is broken — each is an honest test of the 40101 path — but RTN14g is not exercising a
revoked key, so the spec point it is filed under has no coverage. Exercising it wants a key that
exists and has been revoked, and neither the fixture nor the file's Sandbox Setup section
produces one. Same class as #544.

The file also provisions a sandbox app in BEFORE ALL TESTS that neither of its two tests uses:
both connect with credentials for an application that was never created, so the provisioned app has
nothing to do with either connection.

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


1. realtime/integration/connection/connection_failures_test.md:46-53, 91-99 — one fixture, twice

RTN14a (:46-53):

client = Realtime(options: ClientOptions(
  key: "invalid.key:secret",
  endpoint: "nonprod:sandbox",
  autoConnect: false,
  useBinaryProtocol: false
))

RTN14g (:91-99):

# Use a key with a valid format but non-existent app
client = Realtime(options: ClientOptions(
  key: "nonexistent.keyname:keysecret",
  endpoint: "nonprod:sandbox",
  autoConnect: false,
  useBinaryProtocol: false
))

The only difference is the string. invalid and nonexistent are both app IDs that do not exist,
so both connects put the server in the same position, and the test steps that follow are identical
(:56-60 and :102-106).

Measured against the sandbox. The server closes the websocket with a 1008 policy violation and
the SDK reports:

RTN14a  key "invalid.key:secret"            -> FAILED  40101 / 401
        "unable to handle request; no application id found in request"
RTN14g  key "nonexistent.keyname:keysecret" -> FAILED  40101 / 401
        "unable to handle request; no application id found in request"

Both assertion blocks admit that code. RTN14a (:64-67):

ASSERT client.connection.state == FAILED
ASSERT client.connection.errorReason IS NOT NULL
ASSERT client.connection.errorReason.code == 40005 OR client.connection.errorReason.code == 40101
ASSERT client.connection.errorReason.statusCode == 401 OR client.connection.errorReason.statusCode == 404

RTN14g (:110-113):

ASSERT client.connection.state == FAILED
ASSERT client.connection.errorReason IS NOT NULL
# Server returns 40005 (invalid key) or similar non-token error
ASSERT client.connection.errorReason.code < 40140 OR client.connection.errorReason.code >= 40150

40101 satisfies both, so both derived tests pass. The RTN14g section's own note (:86-88) says as
much:

Note: This test uses a syntactically valid but non-existent app ID. The server
rejects the connection with a 404 or 401 error, which is not a token error
(not in the 40140-40149 range), so RTN14g applies.

That is accurate as a statement about which spec point the response falls under. It is not a
revoked key, and it is not a deleted app either — the heading at :74 and the requirement at
:82-84 both claim one:

## RTN14g - Revoked key causes FAILED
**Spec requirement:** When connecting with a key that has been revoked or belongs
to a deleted app, the server sends a non-token ERROR and the connection transitions
to FAILED.

2. realtime/integration/connection/connection_failures_test.md:15-29 — nothing provisioned here can be revoked

BEFORE ALL TESTS:
  response = POST https://sandbox.realtime.ably-nonprod.net/apps
    WITH body from ably-common/test-resources/test-app-setup.json

  app_config = parse_json(response.body)
  api_key = app_config.keys[0].key_str
  app_id = app_config.app_id

AFTER ALL TESTS:
  DELETE https://sandbox.realtime.ably-nonprod.net/apps/{app_id}
    WITH Authorization: Basic {api_key}

ably-common/test-resources/test-app-setup.json is reachable, and its post_apps.keys array holds
six entries: a default key with no restrictions, four differing only in capability, and

{
  "revocableTokens": true
}

That is the file's only revocation affordance, and it enables token revocation, not key
revocation. Revoking a token through POST /keys/{keyName}/revokeTokens was measured against the
sandbox to push {'action': 6, 'error': {'message': 'token revoked', 'code': 40141, 'statusCode': 401}} — inside the 40140–40149 range, which is RTN14b and exactly what RTN14g's assertion excludes.
So nothing in the provisioning body can be used for RTN14g as written.

Separately, api_key and app_id are captured and never read by either test. The provisioned app
is unused: both tests connect with credentials for an application that was never created. Our
derived tests do not request the sandbox app fixture at all for this reason.


Suggested fix

Two ways out, and the choice is yours.

Merge the two sections. They are one test. Keep the RTN14a section, widen its Spec table to
both points since the 40101 response does satisfy RTN14g's "ERROR with an empty channel for reasons
other than token error", delete the RTN14g section at :72-116, and leave genuine key revocation
to a tier that can revoke a key. The Sandbox Setup section at :15-29 can go with it, since
nothing in the file uses the provisioned app.

Or give RTN14g a fixture that produces the condition it names. The cheapest is the deleted-app
half of the requirement, which needs only the two calls the file already makes — provision a second
throwaway app, capture its key, delete the app, then connect with the now-dead key:

BEFORE THIS TEST:
  # A second app, provisioned only to be deleted, so the key under test is one the
  # server issued rather than one that never existed.
  doomed = POST https://sandbox.realtime.ably-nonprod.net/apps
    WITH body from ably-common/test-resources/test-app-setup.json
  doomed_key = doomed.keys[0].key_str

  DELETE https://sandbox.realtime.ably-nonprod.net/apps/{doomed.app_id}
    WITH Authorization: Basic {doomed_key}

client = Realtime(options: ClientOptions(
  key: doomed_key,
  endpoint: "nonprod:sandbox",
  autoConnect: false,
  useBinaryProtocol: false
))

Note that this may well return the same 40101 as the current fixture, in which case the two
sections really are one test and the merge is the honest answer. Key revocation proper —
a key that exists and has been revoked — has no affordance in
ably-common/test-resources/test-app-setup.json and would need a new provisioning step there
before the specification could call for it.

Either way, :82-88 should stop describing a condition the fixture does not create. If the merge
is chosen, the surviving section's requirement text should say what is actually provoked: a
connection attempt with credentials for an application that does not exist, answered with a
non-token error on an empty channel.

Not blocking us. Both tests are derived as written and both pass, in
test/uts/realtime/integration/connection/connection_failures_test.py, with a module docstring
recording that the two provoke one response. We are not carrying a workaround; we are recording
that a spec point is uncovered.

Found while deriving uts/realtime/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