fix(client): re-send the metrics schema after a reconnect - #101
Open
Frando wants to merge 1 commit into
Open
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-services/pr/101/docs/iroh_services/ Last updated: 2026-08-11T23:03:47Z |
A client's metrics silently stop being recorded after any reconnect (a server restart, a network change) and never recover until the client process itself restarts. The server keeps its metrics decoder per connection, while the client's encoder lives for the process and includes the schema in an export only when its own registry version changed. The rpc connection is lazy and re-dials transparently, so after a reconnect the client keeps exporting updates whose values the server's fresh decoder cannot name, and they are dropped while the endpoint still shows online. Every reconnect funnels through a real re-auth within one failed round trip, because the server refuses any non-Auth first request on a new connection. So auth() now rebuilds the encoder whenever it actually performs the handshake: a fresh encoder starts at schema version zero and its next export carries the full schema for the server's fresh decoder. Re-importing a schema is idempotent on the server, so re-sending after a spurious re-auth is harmless. Rpc failures on every path (metrics, ping, naming, capability grants, diagnostics) now mark the session unauthorized, where previously only the metrics and ping paths did, so any of them re-arms the handshake. The round-trip test runs a recording stand-in of the backend with the same session rules (Auth first per connection, per-connection decoder), kills the connection mid-run, and asserts the first update after the re-dial carries the schema and decodes; it fails without the encoder rebuild. A second test pins the encoder and decoder contract the fix relies on.
Frando
force-pushed
the
fix/metrics-reconnect-schema
branch
from
August 11, 2026 23:02
a71b6f8 to
1740b80
Compare
Contributor
|
Think this bug was encountered and "addressed" before somewhere, but beats me to find the actual trail of it. I've dealt with this at some point.... |
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.
Description
iroh-services kept the metrics encoder global per client. The metrics encoder sends the schema only once, then only value updates (on purpose, to save bandwidth). However this means it needs to be recreated if the irpc connection breaks and reconnects, because otherwise the new connection receives only values but not the initial schema.
This PR fixes this by recreating the encoder on reconnect, so that it will send the schema again.
Without this fix, metrics were stopping to work whenever the connection to svc broke because svc would ignore metrics pushed on a connection where a schema was never included.
Comes with a test that fails without the fix.
Breaking Changes
Notes & open questions
Change checklist