Skip to content

fix: parse FDv2 goodbye events with only a reason field#380

Merged
keelerm84 merged 1 commit intomainfrom
mk/sdk-2296/remove-cat-silence
Apr 30, 2026
Merged

fix: parse FDv2 goodbye events with only a reason field#380
keelerm84 merged 1 commit intomainfrom
mk/sdk-2296/remove-cat-silence

Conversation

@keelerm84
Copy link
Copy Markdown
Member

@keelerm84 keelerm84 commented Apr 30, 2026

Summary

  • The FDv2 goodbye event is specified to carry only a reason field. The Ruby SDK's Goodbye class incorrectly declared silent and catastrophe, and from_h raised ArgumentError if either was missing — meaning a spec-compliant payload would fail to parse.
  • This PR removes both fields from the class, the constructor, to_h, and from_h. from_h now requires only reason.
  • Streaming consumer in lib/ldclient-rb/impl/data_system/streaming.rb was logging at @logger.error and reading goodbye.silent / goodbye.catastrophe. Replaced with @logger.info { "[LDClient] SSE server received goodbye: #{goodbye.reason}" } — a goodbye is a normal protocol signal, matching the Java SDK.

Test plan

  • bundle exec rspec spec/impl/data_system/streaming_synchronizer_spec.rb — 14 examples, 0 failures.
  • bundle exec rspec spec/impl/data_system/ — 125 examples, 0 failures.
  • grep -rn for goodbye.silent / goodbye.catastrophe — no remaining references.

Note

Low Risk
Low risk: narrows Goodbye parsing/serialization to the spec-required reason field and adjusts logging; behavior change is limited to handling GOODBYE SSE events.

Overview
Aligns FDv2 Goodbye event handling with the protocol by removing the unsupported silent/catastrophe fields from ProtocolV2::Goodbye (constructor, to_h, and from_h now only require reason).

Updates the streaming SSE consumer to stop treating GOODBYE as an error condition and instead log it at info, and adjusts the streaming synchronizer spec to construct goodbye events with only reason.

Reviewed by Cursor Bugbot for commit 17ff311. Bugbot is set up for automated code reviews on this repo. Configure here.

The FDv2 goodbye event spec defines only a reason field, but the Goodbye
class required silent and catastrophe — causing from_h to raise
ArgumentError on spec-compliant payloads. Drop the extra fields and
simplify the streaming handler to log the reason unconditionally.
@keelerm84 keelerm84 requested a review from a team as a code owner April 30, 2026 15:21
@keelerm84 keelerm84 merged commit 4f5a396 into main Apr 30, 2026
12 checks passed
@keelerm84 keelerm84 deleted the mk/sdk-2296/remove-cat-silence branch April 30, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants