Skip to content

Add optional on_error/4 callback to SessionHandler#23

Open
santif wants to merge 5 commits intomasterfrom
feat/callback-on-error
Open

Add optional on_error/4 callback to SessionHandler#23
santif wants to merge 5 commits intomasterfrom
feat/callback-on-error

Conversation

@santif
Copy link
Copy Markdown
Owner

@santif santif commented Mar 5, 2026

Summary

  • Adds an optional on_error/4 callback to the SessionHandler behaviour that notifies handlers of session-level errors
  • Supports four error types: :connect_error, :transport_error, :garbled_message, and :heartbeat_timeout
  • Fully backwards-compatible — handlers that don't implement the callback are unaffected
  • Callback exceptions are rescued and logged without crashing the session

Error types

Error type Source Details
:connect_error SessionWorker — TCP/SSL connect fails %{reason: term()}
:transport_error SessionWorkertransport.send fails %{reason: term()}
:garbled_message Session — checksum/body-length validation fails %{raw_message: binary()}
:heartbeat_timeout Session — second consecutive RX timeout %{}

Test plan

  • Handler with on_error/4 receives :garbled_message on garbled FIX messages
  • Handler without on_error/4 does not crash on garbled messages
  • :connect_error dispatched when transport.connect fails
  • :transport_error dispatched when transport.send returns error
  • :heartbeat_timeout dispatched on second consecutive RX timeout
  • Raising on_error/4 handler is rescued without crashing the session
  • mix credo and mix dialyzer pass with no new warnings

Note

Medium Risk
Touches core session and transport code paths (timeouts, invalid message handling, and socket send/connect), which could affect runtime behavior under error conditions. The callback is optional and guarded/rescued, reducing risk but still introducing new side effects and logging in failure paths.

Overview
Adds a new optional error-notification callback ExFix.SessionHandler.on_error/4 (declared via @optional_callbacks) to let applications react programmatically to session/transport failures.

Session now invokes on_error/4 for :garbled_message and :heartbeat_timeout (second consecutive RX timeout) with guarded function_exported?/3 checks and try/rescue logging. SessionWorker now checks transport.send/2 results and notifies :transport_error, and also notifies :connect_error when socket connect fails, via a shared maybe_notify_error/5 helper.

Adds/updates test handlers and new unit tests covering each error type, optional-callback behavior, and handler exceptions; also adds OpenSpec specs/archive docs plus small repo hygiene updates (CLAUDE.md, .gitignore for /.serena/).

Written by Cursor Bugbot for commit 9811156. This will update automatically on new commits. Configure here.

Santiago Fernandez and others added 4 commits March 4, 2026 16:56
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add OpenSpec change artifacts for the on_error/4 optional callback on
SessionHandler. Session calls on_error directly for protocol errors
(garbled messages, heartbeat timeouts), SessionWorker for transport
errors (connect/send failures).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduce an optional `on_error/4` callback that notifies handlers of
session-level errors: :connect_error, :transport_error, :garbled_message,
and :heartbeat_timeout. The callback is guarded with function_exported?
and wrapped in try/rescue so handlers that don't implement it are
unaffected and raising handlers can't crash the session.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Free Tier Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Logger.error(fn ->
"[fix.error] [#{session_name}] on_error callback raised: #{inspect(e)}"
end)
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated error notification pattern across session module

Low Severity

The function_exported? + try/rescue + Logger.error pattern for invoking on_error is copy-pasted twice inline in session.ex (for :heartbeat_timeout and :garbled_message), while session_worker.ex correctly extracts the identical logic into a reusable maybe_notify_error/5 helper. A similar private helper in session.ex would eliminate the duplication and keep the error-handling contract consistent across both modules.

Additional Locations (2)

Fix in Cursor Fix in Web

Archive the completed callback-on-error change (16/16 tasks done) and
sync the error-callback delta spec to openspec/specs/.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant