restore the IRS success event that was lost in an April 2024 merge - #1110
Merged
Conversation
The IRS success branch set ENDED and returned without the session-finished event, while abort, failure and the ISS success path all send one. A successful inbound raw transfer was invisible to every websocket/REST consumer, and the received payload was dropped with it (handle_raw returns the data into a call chain that discards it). c2388a6 emitted exactly this event, data= included; it vanished by ef18f4c without that commit's IRS diff touching the lines, so this looks like a merge casualty rather than a decision. Restore the event and add a regression test: one finished event with success=True, payload round-trips through the base64 data field. The original also pushed session statistics gated on enable_stats, but that key is no longer in the config schema (config.py STATION), so the strict lookup raises KeyError; the two existing stats sites in the failure and abort paths have the same latent problem. Restoring just the event here; the stats question is worth its own look. No setARQ(False) here: dispatch() already clears busy on this path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
With #1109, a raw ARQ transfer completes over the air, but the
receiving station never announces it. The success branch in
arq_session_irs.pysets ENDED and returns withoutsend_arq_session_finished, while the abort path, the failure path,and the sender's success path all emit one. There's no way for a
websocket or REST consumer to detect a successful inbound raw
transfer, and the received payload is dropped with it (
handle_rawreturns the data into a call chain that ignores the return value).
Full story in #1108.
This used to work. c2388a6 ("bringing back statistics", 2024-04-02)
emitted exactly this event,
data=included, and it disappeared byef18f4c (2024-04-07) even though that commit's IRS diff doesn't touch
those lines. It looks like a merge casualty rather than a decision, so
this PR puts the event call back:
One deliberate difference from c2388a6: the original also pushed the
session statistics to the stats server, gated on
config["STATION"]["enable_stats"]. That key is still inconfig.ini.example but no longer in config.py's STATION schema, so the
strict lookup raises KeyError today, and the two existing stats pushes
in the IRS failure and abort paths have the same latent problem. So
this PR restores just the event and leaves the stats question alone;
I flagged it separately in #1108.
A few notes:
outbound=Falsematches the IRS abort path. The IRS failure pathpasses
Truethere, which looks like its own small inconsistency;I left it alone here and can send a follow-up if you'd like it
aligned.
setARQ(False)added:dispatch()already clears busy on thispath.
finished event with
success=True, and the payload round-tripsthrough the event's base64
datafield.message_received), which is presumably why this went unnoticedfor two years.
eventHandler.jshas anexplicit
case "ENDED"handler forarq-transfer-inboundthat hasbeen dead code since the event stopped arriving. This PR brings the
receive-side completion UI back to life along with the API.
This branch is independent of #1109 (both sit directly on develop);
they can land in either order.