fix: deliver provider events in order, outside handler locks by josecolella · Pull Request #351 · open-feature/ruby-sdk

josecolella · 2026-10-23T16:39:17Z

Summary

Fixes Reentrant provider events reach client callbacks out of order #332. When a handler makes a provider emit another event, that event is now delivered after the current one finishes, in order, so client callbacks no longer arrive out of order.
Client event handlers now run outside the lock that guards them. They can add or remove handlers and emit events without the deadlock; recursive locking error being logged.
Part of Spec v0.9.0 compliance #300 (spec 5.1.2, 5.2.7, 5.3.3, 5.3.5).

Behavior changes worth noting

Ordering: an event for a provider that is already being dispatched on the same call stack is queued behind the current one. Events for other providers still dispatch immediately. There's no ordering across threads.
When status is recorded: a provider-emitted event records its status when it's delivered, so the outer event's handlers still see the outer status (5.3.5). The SDK's own READY/ERROR from init records its status when sent. That way set_provider_and_wait called from the provider's own handler (for example, an ERROR handler retrying init) returns with the provider READY.
Stale events: a READY/ERROR that the SDK sent for an earlier registration of the same provider instance is dropped.
client.add_handler now raises ArgumentError for an unknown event type or a missing handler, the same as the API-level add_handler. Before, a missing handler was accepted and only failed when an event fired.
Immediate handlers: an API handler added after a provider is READY runs once for that provider, not once for each domain it's bound to (5.3.3).
Failing queued event: if a queued event fails inside the SDK (for example, non-Hash details), the failure is logged as "Queued provider event failed". It no longer fails the emit or init that was dispatching.
Known edges:
- A handler that re-emits its own event for the same provider now loops forever. On main it raises SystemStackError.
- An exception that isn't a StandardError, raised by a handler, drops the events queued behind it on that call stack.

Merging

Squash merges here use an empty body. Add Co-authored-by: Leo Romanovsky <57361+leoromanovsky@users.noreply.github.com> to the squash message, since Reentrant provider events reach client callbacks out of order #332's reproduction and proposed direction are his.

🤖 Generated with Claude Code

- An event emitted for a provider that is already being dispatched on this call stack is queued and delivered after the current one, in order (#332). Events for other providers dispatch right away. - Provider events record their status when delivered (5.3.5). The SDK's own READY/ERROR records it when sent, so set_provider_and_wait from the provider's own handler returns with the provider READY. - A READY/ERROR the SDK sent for an earlier registration of the same provider is dropped. - Client handlers run outside the client-handler lock, so they can add or remove handlers and emit events. add_client_handler validates like the API, through one shared validator. - A queued event that fails is logged instead of failing the emit or init that was dispatching. - An API handler added after the fact runs once per provider, not once per bound domain. Signed-off-by: Jose Colella <jose.colella@gusto.com> Co-authored-by: Leo Romanovsky <57361+leoromanovsky@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-23T16:39:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ 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: 01cc8762-5b9e-42ba-bcea-81a32b30b033

📥 Commits

Reviewing files that changed from the base of the PR and between 62759f4 and ca4ab53.

📒 Files selected for processing: 6

lib/open_feature/sdk/configuration.rb
lib/open_feature/sdk/event_dispatcher.rb
sig/open_feature/sdk/configuration.rbs
sig/open_feature/sdk/event_dispatcher.rbs
spec/open_feature/sdk/configuration_spec.rb
spec/specification/events_spec.rb

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

Provider lifecycle events now carry initialization-attempt tokens. Configuration rejects stale token-bearing events and queues same-thread reentrant events for ordered delivery. Client handler invocation and immediate API-handler checks also change.

Changes: Provider event lifecycle and delivery

Layer / File(s)	Summary
Initialization-attempt tokens `lib/open_feature/sdk/configuration.rb`, `sig/open_feature/sdk/configuration.rbs`, `spec/specification/events_spec.rb`	READY and ERROR events include initialization-attempt tokens. Tests cover nested retry and concurrent initialization attempts.
Ordered provider event dispatch `lib/open_feature/sdk/configuration.rb`, `sig/open_feature/sdk/configuration.rbs`, `spec/open_feature/sdk/configuration_spec.rb`, `spec/specification/events_spec.rb`	Configuration rejects stale token-bearing events and queues same-thread reentrant events for ordered delivery. Tests cover queued events, status visibility, and thread-key cleanup.
Handler registration and invocation `lib/open_feature/sdk/configuration.rb`, `lib/open_feature/sdk/event_dispatcher.rb`, `sig/open_feature/sdk/event_dispatcher.rbs`, `spec/specification/events_spec.rb`	Client handler registration validates event types and handlers. Client callbacks run outside the handler mutex. Immediate API callbacks check each distinct bound provider once.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Provider
  participant Configuration
  participant APIHandler
  participant ClientHandler
  Provider->>Configuration: Emit outer provider event
  Configuration->>APIHandler: Invoke API-level handler
  APIHandler->>Provider: Emit nested provider event
  Provider->>Configuration: Dispatch nested event
  Configuration->>Configuration: Queue nested event
  Configuration->>ClientHandler: Deliver outer event
  Configuration->>APIHandler: Deliver queued event
  Configuration->>ClientHandler: Deliver queued event

Possibly related issues

open-feature/ruby-sdk#300 — The issue includes ordered event delivery and handler reentrancy, which this change modifies.
open-feature/ruby-sdk#344 — The Multi-Provider issue identifies ordered event delivery and handler locking as dependencies; this change modifies both.

Possibly related PRs

open-feature/ruby-sdk#348 — Both changes modify provider initialization attempts; this PR uses those attempts to validate READY and ERROR events.

Suggested reviewers: sameerank

Merge Risk: 🔵 Low · up to ca4ab

The change is mergeable with awareness that an event queued before shutdown can reach a newly bound registration of the same provider object. This narrow lifecycle edge can be addressed in a follow-up.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The pull request also changes behavior that `#332` does not require: client-handler execution outside its mutex, handler validation, API immediate-handler deduplication, initialization-token status timi…	Remove these unrelated behavior changes and their tests from this pull request, or place them in a separate pull request with the applicable active issue linked.
Docstring Coverage		Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description check		The description directly explains the provider event ordering, handler locking, lifecycle behavior, validation changes, and related tests.
Title check		The title clearly and concisely summarizes the main changes: ordered provider-event delivery and dispatch outside handler locks.
Linked Issues check		The implementation in `Configuration#dispatch_provider_event` queues reentrant events per provider and thread, then drains the queue after the current event finishes. The queue preserves event order a…

Full details: Linked Issues check

Explanation

The implementation in Configuration#dispatch_provider_event queues reentrant events per provider and thread, then drains the queue after the current event finishes. The queue preserves event order and copied details. accept_provider_event rejects events after provider shutdown or replacement. The added event specifications cover nested ordering, payload preservation, and replacement invalidation. This satisfies the coding requirements in #332. No cross-thread ordering is required by #332.

Full details: Out of Scope Changes check

Explanation

The pull request also changes behavior that #332 does not require: client-handler execution outside its mutex, handler validation, API immediate-handler deduplication, initialization-token status timing, and queued-event failure logging. The linked issue only requires ordered reentrant provider delivery and invalidation after registration ends. The summary does not identify a directly linked active issue that supplies requirements for these additional changes.

Resolution

Remove these unrelated behavior changes and their tests from this pull request, or place them in a separate pull request with the applicable active issue linked.

Full details: Docstring Coverage

Explanation

Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (2 skipped: 2 unsupported.)

Resolution

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

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

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

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

codecov · 2026-10-23T16:39:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.66%. Comparing base (62759f4) to head (ca4ab53).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
+ Coverage   99.54%   99.66%   +0.11%     
==========================================
  Files          32       32              
  Lines         887      906      +19     
==========================================
+ Hits          883      903      +20     
+ Misses          4        3       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Jose Colella <jose.colella@gusto.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

🟡 Changes recommended

Deferred delivery still has correctness gaps in provider status, registration invalidation, and mutable payload preservation.

3 open issues

_{Review effort: Balanced}

-          @provider_state_registry.update_state_from_event(provider, event_type, details)
+        queues = (Thread.current[@dispatch_key] ||= {}.compare_by_identity)
+        if (queue = queues[provider])
+          queue << [event_type, details.dup, token]


-          provider_name: provider_name
-        }.merge(details)
+      def deliver_provider_event(provider, event_type, details, token)
+        return unless accept_provider_event(provider, event_type, details, token, record_status: token.nil?)


+        @provider_mutex.synchronize do
+          return false unless provider_bound?(provider)
+          # An SDK READY/ERROR from an earlier registration of this provider is stale
+          return false unless token.nil? || @init_attempts[provider].equal?(token)


josecolella mentioned this pull request Oct 23, 2026

Reentrant provider events reach client callbacks out of order #332

Closed

test: cover a client handler that raises

ca4ab53

Signed-off-by: Jose Colella <jose.colella@gusto.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

josecolella marked this pull request as ready for review October 23, 2026 16:57

josecolella requested a review from a team as a code owner October 23, 2026 16:57

Copilot AI balanced review requested due to automatic review settings October 23, 2026 16:57

Copilot AI reviewed Oct 23, 2026

josecolella mentioned this pull request Oct 23, 2026

feat: let providers own their lifecycle events #353

Merged

josecolella requested a balanced review from Copilot October 23, 2026 17:31

Copilot started reviewing on behalf of josecolella October 23, 2026 17:32

View session

josecolella merged commit 16bf89c into main Oct 23, 2026
28 checks passed

josecolella deleted the fix/ordered-event-delivery branch October 23, 2026 17:32

openfeaturebot mentioned this pull request Oct 23, 2026

chore(main): release 0.7.0 #350

Merged

Copilot AI reviewed Oct 23, 2026

View reviewed changes

josecolella mentioned this pull request Oct 23, 2026

Spec v0.9.0 compliance #300

Closed

17 tasks done

coderabbitai Bot mentioned this pull request Oct 23, 2026

fix: close the spec v0.9.0 gaps the compliance audit found #361

Merged

Navigation Menu

Repository navigation

fix: deliver provider events in order, outside handler locks - #351

fix: deliver provider events in order, outside handler locks#351
josecolella merged 2 commits into
mainfrom
fix/ordered-event-delivery

josecolella commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

codecov Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

josecolella commented Oct 23, 2026

Summary

Behavior changes worth noting

Merging

Uh oh!

coderabbitai Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

codecov Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

codecov Bot commented Oct 23, 2026 •

edited

Loading