Repository navigation
fix: give StateMachine, Client and Manager one protocol policy - #148
Merged
Merged
Conversation
Since the three front ends moved onto the shared protocol core, fifteen Manager-only core options and two Manager adapter remaps kept Manager's older error and correlation policies where StateMachine and Client did something else. Twelve options and both remaps are gone, so each input now gets one disposition in every front end. The three options that remain decide who closes the connection after a message error: the core for natively admitted StateMachine and Client connections, the reader for Manager and externally admitted ones. The core's own phase and timers follow from that choice: one decision marks the connection terminal and selects whether a required reply closes it once written, and the write's completion then terminates it. The core options are now documented that way. Behaviour changes, by the front end whose users see them: - StateMachine and Client answer a repeated CER that carries a fatal decoding error, such as an invalid AVP length, with an error CEA before they close, as Manager did, so the peer learns why the connection ends (RFC 6733 section 7). Settings.OnCEA runs for that error CEA. A repeated CER without a fatal decoding error is still ignored. - StateMachine and Client keep a fatal decoder error fatal when a base header error decides the Result-Code: they send the answer a request requires, report Closing and close, as for every other fatal message error. - StateMachine and Client report a fatal error received during Closing as fatal; Closing timers, replies and the connection are unchanged. - StateMachine and Client check a DWA's Origin-Host and Origin-Realm before its Result-Code, as DPA matching does, so an unsuccessful DWA from the wrong peer is reported as invalid, not as a rejected probe. - StateMachine, Client and Manager report a well-formed DWA that answers no outstanding DWR as an unexpected message with Result-Code 0 and the correlation detail (RFC 6733 section 3). StateMachine and Client log it as "sm: DWA identifiers mismatch; answer discarded" and no longer call the invalid-answer watchdog hook for it. - Client and dialed Manager discard a CEA whose identifiers do not match the pending CER as an unexpected message (RFC 6733 section 3). Manager no longer fails the handshake with a CEA rejection for it. If the CEA also carries a fatal decoding error, the connection still ends: Client's core fails the handshake and closes, Manager's reader closes, and both report Closing. Without a fatal error the pending CER keeps its deadline. - Dialed Manager discards a CEA that arrives after the handshake by ownership, silently for an exact duplicate, instead of validating it. - Manager reports a DPA that answers no outstanding DPR during Closing as an unexpected message rather than InvalidDPA, as StateMachine and Client do. - Manager derives the header Result-Code for a decoded first message that is not a CER, and keeps the decoder's code for a reported decoding error. - Manager classifies an invalid-header request during Closing as InvalidHeader rather than MalformedMessage. - Manager resets Tw when it receives a message with a valid header and a fatal body error, without watchdog credit (RFC 3539 section 3.4.1). - Manager.Close rejects a Disconnect-Cause outside 0-2 (RFC 6733 section 5.4.3) before any shutdown step, also on a Manager that is already closed, with the error StateMachine.Disconnect returns. - StateMachine and Client keep the standard fmt wrapper around a failed error-answer build in the error they record. New TCP parity tests cover each change through StateMachine (accepted and Client) and Manager (accepted and dialed). A run of 6,100 shared scenarios against main differs only where listed above, and bytes and allocations per exchange are at or below main. Closes #143.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since the three front ends moved onto the shared protocol core, fifteen
Manager-only core options and two Manager adapter remaps kept Manager's
older error and correlation policies where StateMachine and Client did
something else. Twelve options and both remaps are gone, so
each input now gets one disposition in every front end. The three
options that remain decide who closes the connection after a message
error: the core for natively admitted StateMachine and Client
connections, the reader for Manager and externally admitted ones. The
core's own phase and timers follow from that choice: one decision marks
the connection terminal and selects whether a required reply closes it
once written, and the write's completion then terminates it. The core
options are now documented that way.
Behaviour changes, by the front end whose users see them:
decoding error, such as an invalid AVP length, with an error CEA
before they close, as Manager did, so the peer learns why the
connection ends (RFC 6733 section 7). Settings.OnCEA runs for that
error CEA. A repeated CER without a fatal decoding error is still
ignored.
header error decides the Result-Code: they send the answer a request
requires, report Closing and close, as for every other fatal message
error.
as fatal; Closing timers, replies and the connection are unchanged.
before its Result-Code, as DPA matching does, so an unsuccessful DWA
from the wrong peer is reported as invalid, not as a rejected probe.
no outstanding DWR as an unexpected message with Result-Code 0 and the
correlation detail (RFC 6733 section 3). StateMachine and Client log
it as "sm: DWA identifiers mismatch; answer discarded" and no longer
call the invalid-answer watchdog hook for it.
the pending CER as an unexpected message (RFC 6733 section 3). Manager
no longer fails the handshake with a CEA rejection for it. If the CEA
also carries a fatal decoding error, the connection still ends:
Client's core fails the handshake and closes, Manager's reader closes,
and both report Closing. Without a fatal error the pending CER keeps
its deadline.
ownership, silently for an exact duplicate, instead of validating it.
as an unexpected message rather than InvalidDPA, as StateMachine and
Client do.
that is not a CER, and keeps the decoder's code for a reported
decoding error.
InvalidHeader rather than MalformedMessage.
fatal body error, without watchdog credit (RFC 3539 section 3.4.1).
5.4.3) before any shutdown step, also on a Manager that is already
closed, with the error StateMachine.Disconnect returns.
error-answer build in the error they record.
New TCP parity tests cover each change through StateMachine (accepted
and Client) and Manager (accepted and dialed). A run of 6,100 shared
scenarios against main differs only where listed above, and bytes and
allocations per exchange are at or below main.
Closes #143.
Summary by cubic
Consolidates the error and correlation policy across StateMachine, Client, and Manager by removing fifteen Manager-only core options and two adapter remaps; each input now gets one disposition in every frontend. Only the ownership of connection closure after a message error still differs: the core for natively admitted StateMachine and Client connections, the reader for Manager and externally admitted ones.
Behavior changes users will see
Manager.Closerejects a Disconnect-Cause outside 0–2 before any shutdown step, also on a Manager that is already closed.New TCP parity tests cover each change; a run of 6,100 shared scenarios differs from main only in the listed behavior, at or below main's bytes and allocations per exchange.
Closes #143.
Written for commit 7160472. Summary will update on new commits.