[RaphaelFakhri]

Description

When RESTChannel.Presence.History receives a presence message whose data can't be decoded (for example, an unknown encoding), the SDK logs the failure with %w. The logger formats messages with fmt.Sprintf, which doesn't support %w, so the log line ends with %!w(*errors.errorString=&{unknown encoding nonsense}) instead of the error text.

This change uses %v, which matches the equivalent messages in rest_channel.go.

Testing

Adds TestRESTPresence_HistoryLogsDecodeFailure. It serves a presence history page with an unknown encoding from a local HTTP server, records the log output through WithLogHandler, and asserts that the decode failure is logged without a formatting error and includes the error text.

go test ./ably -run TestRESTPresence_HistoryLogsDecodeFailure -count=1

Without the fix, the test fails with a log line that contains %!w(. With the fix, it passes.

Summary by CodeRabbit

• Bug Fixes
  • Improved error logging when presence-history responses use an unknown encoding.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The presence-message decode-error log now formats errors with %v. A new non-integration test checks the log output when presence history contains an unknown encoding.

Changes

Presence decode log

Layer / File(s)	Summary
Decode log formatting and test internal/ably/http_presence.go, internal/ably/http_presence_test.go	The decode-error log uses %v. The test fetches presence history with an unknown encoding and checks the logged message and formatting.

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

Change: Bug fix

Suggested reviewers: tcard

Merge Risk: Minimal · up to a9b4a

The change corrects malformed presence-history error logs without changing decoding behavior. It is mergeable subject to normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the primary change: the decode-failure log line for presence history is fixed so it formats correctly. The raw summary confirms the one-line change from %w to %v in http_presence.go, a…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the primary change: the decode-failure log line for presence history is fixed so it formats correctly. The raw summary confirms the one-line change from %w to %v in http_presence.go, and the objectives confirm the fix targets malformed output such as %!w(...) when RESTChannel.Presence.History hits undecodable data. The added test is a supporting part of the same change and does not need to appear in the title.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit heard the log say “hello,”
No formatting warning stole the show.
One odd encoding hopped in sight,
The test caught its name just right.
Then carrots crunched beneath the moon.}

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f4ddc992-00c2-4894-bdac-bee1b760167f

Commits

Reviewing files that changed from the base of the PR and between 41b41cc and a9b4a12.

Files selected for processing: 2

• internal/ably/http_presence.go
• internal/ably/http_presence_test.go

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

[RaphaelFakhri]

Rebased onto main after the 2.0.0 restructure. The fix is now in internal/ably/http_presence.go, and the test moved to internal/ably/http_presence_test.go with the new import path and NewHTTPClient. It fails on main with %!w(...) in the log line and passes with the fix.