Skip to content

fix: report error code 80017 for a closed connection - #1252

Open
RaphaelFakhri wants to merge 1 commit into
ably:mainfrom
RaphaelFakhri:fix/connection-closed-error-code
Open

RaphaelFakhri wants to merge 1 commit into
ably:mainfrom
RaphaelFakhri:fix/connection-closed-error-code

Conversation

@RaphaelFakhri

@RaphaelFakhri RaphaelFakhri commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #797

Description

When the connection is closing or closed, the SDK reports ErrorInfo("Can't attach when not in an active state", 200, 10000). Error code 10000 means "no error" in the Ably error list, and the HTTP status code is 200. This error appears in two places:

  • The reason of the closing and closed connection state changes, and of the detached channel state change that follows a close.
  • The error passed to attach() on a channel after the connection closes.

This change reports ErrorInfo("Connection closed", 400, 80017) instead. Code 80017 is the "connection closed" code in the Ably error list, and ably-js reports the same code and status code for the closing and closed states.

Testing

ConnectionManagerClosedErrorTest connects through a mock transport that refuses connections, closes the connection, and asserts code 80017 and statusCode 400 on the closed state change and on a later attach(). It fails without the change (Expected: is <80017> but: was <10000>) and passes with it.

Run it with:

./gradlew :java:test --tests io.ably.lib.transport.ConnectionManagerClosedErrorTest

Summary by CodeRabbit

  • Bug Fixes
    • Corrected the reason reported when a connection closes, including its message and error codes. Channels attached afterward now receive the same closed-connection error details.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e14250e5-db4e-48d9-a778-0985f37ea34f

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7d3f5 and 0a86600.


📒 Files selected for processing (2)
  • lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
  • lib/src/test/java/io/ably/lib/transport/ConnectionManagerClosedErrorTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



Walkthrough

The closed connection reason now uses the message "Connection closed", status 400, and error code 80017. A test checks those values in the closed state and when attaching a channel.

Changes

Closed Connection Error

Layer / File(s) Summary
Closed reason and validation
lib/src/main/java/io/ably/lib/transport/ConnectionManager.java, lib/src/test/java/io/ably/lib/transport/ConnectionManagerClosedErrorTest.java
REASON_CLOSED now uses message "Connection closed", status 400, and error code 80017. The test checks the closed-state reason and the channel attachment error.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: ttypic

Merge Risk: ⚪ Minimal · up to 0a866

The change aligns the reported closed-connection error with the intended code and status. It is ready to merge after normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: reporting error code 80017 for a closed connection.
Linked Issues check Passed Issue [#797] requires correction of the invalid 10000 code for REASON_CLOSED. The pull request changes ConnectionManager.REASON_CLOSED from `ErrorInfo("Can't attach when not in an active state",…
Out of Scope Changes check Passed The pull request changes one connection error declaration and adds one focused regression test. Both changes directly support issue [#797]. No unrelated production behavior or unrelated test changes a…

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the closed-state code,
And hops along the channel road.
Four hundred marks the status clear,
Eight-zero-zero-one-seven appears.
The test confirms the reason true,
Then nibbles clover in the queue.

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Invalid error code 10000 emitted for REASON_CLOSED "Can't attach when not in an active state"

1 participant