Separate Observe registration answers from pushes - #80
Merged
Conversation
A device answers every OBSERVE register CON with the current representation, and that answer reaches on_notification exactly as a spontaneous notification does. The bridge counted it as push, so Push Active read online for its whole ten-minute window on an appliance with no route to Samsung's cloud, which emits nothing at all. RFC 7641 3.2 makes the first response on a token the answer to the registration, and the session already resolves the token, the Message ID and the Observe option. None of that reaches a caller: on_notification carries (href, payload) alone, so a consumer can only guess from arrival order, and guessing breaks on an href carrying several query-qualified relations. ObserveDelivery carries the whole relation context, and on_observe_delivery takes precedence over on_notification and on_legacy_notification so a representation is still delivered once. The flag travels with a queued blockwise relation, so a re-read stays a registration answer. Retiring a token clears its recorded sequence, which makes a reconnect or the periodic refresh register afresh with no bookkeeping in the caller. Existing consumers are untouched: the callback defaults to None and on_notification keeps receiving every representation. Confirmed on hardware. The previous build put both reference appliances online 55s after connect and held them there for the full window; this one logs no push_active online event in 35 minutes, with both units at 0 err and 0 timeouts and no state change sourced from observe at all. Closes #41 Claude-Session: https://claude.ai/code/session_018zWZfikEW9X1RVyadNZD3C
connect() turned every SSL.Error into a bare SessionError and discarded the exception, so a rejected handshake reached the operator as "session operation failed" and nothing else. A reference appliance did that once during a deploy and recovered on its own retry, and the alert that would have explained it was already gone. The redaction is deliberate and stays: the errors module refuses arbitrary detail because backend errors can carry remote endpoints, local paths or credential metadata. So the alert goes to the local log instead, narrowed to the reason strings OpenSSL recorded. Those are protocol vocabulary, "tlsv1 alert unknown ca" and the like, and name the failure without naming the peer. A path or a host sitting in another slot of the reason tuple is dropped, which a test pins. The raised SessionError is unchanged. Claude-Session: https://claude.ai/code/session_018zWZfikEW9X1RVyadNZD3C
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.
Two commits. The first closes #41, the second came out of validating it on hardware.
Registration answers are not pushes (#41)
Push Activewent online for its full ten-minute window on every connect, including on an appliance with no route to Samsung's cloud that emits nothing. The cause is that a device answers every register CON with the current representation, and that answer reacheson_notificationexactly as a spontaneous notification does.RFC 7641 §3.2 makes the first response on a token the answer to the registration, and the session already resolves the token, the Message ID and the Observe option. None of it reaches a caller, because
on_notificationcarries(href, payload)alone. A consumer can only infer from arrival order, and that inference breaks on an href carrying several query-qualified relations, since both answers look alike.ObserveDeliverycarries the relation context andon_observe_deliveryreceives it, taking precedence overon_notificationandon_legacy_notificationso a representation is still delivered once. The flag travels with a queued blockwise relation, so a re-read stays a registration answer. Retiring a token clears its recorded sequence, so a reconnect or the 6-hourly refresh registers afresh with nothing tracked in the caller.Additive and keyword-only. The callback defaults to
Noneand existing consumers keep receiving every representation throughon_notification, which a test pins.Name the alert when a handshake fails
connect()turned everySSL.Errorinto a bareSessionErrorand dropped the exception, so a rejected handshake read assession operation failedand nothing more. One reference appliance did exactly that during a deploy and recovered on its own retry, with the alert already gone.The redaction stays, because it is deliberate: the errors module refuses arbitrary detail since backend errors can carry remote endpoints, local paths or credential metadata. The alert goes to the local log instead, narrowed to the reason strings OpenSSL recorded. A path or host in another slot of the reason tuple is dropped, which a test pins. The raised
SessionErroris unchanged.This is also the artefact I keep asking reporters for in #16 and #20.
Validation
739 tests pass, and the first commit passes alone at 732 for bisect. Each new behaviour is mutation-checked: reverting the registration flag, the optionless case, its survival across a blockwise refetch, and the log line each fail exactly the test written for them.
On hardware, against my oven and dryer. The previous build put both appliances online 55s after connect and held them for the full window. This one logged no
push_activeonline event across 35 minutes, with both units at 0 err, 0 ping-fail and 0 timeouts, and no state change sourced from observe at all.The handshake failure did not reproduce across a graceful restart, a rebuild and recreate, or a SIGKILL with no close_notify. The SIGKILL case settles something else in passing: a bridge killed dead, leaving an orphaned association on the device, re-handshaked on the same 5-tuple and was accepted first time on both appliances. That is the RFC 6347 §4.2.8 behaviour the fixed local port depends on, checked before only on the oven. So the cause of the original rejection is still open, and the logging is there to name it next time.
https://claude.ai/code/session_018zWZfikEW9X1RVyadNZD3C