fix(signal): forward-compat — log+continue on unknown SignalMessage variants

Both sides of the signal channel previously broke their recv loop
on any deserialize error, which meant adding a new variant in one
build silently killed signal connections from peers running an
older build. This bit us during Phase 1 testing: a new client
sending SignalMessage::Reflect to a pre-Phase-1 relay caused the
relay to drop the whole signal connection, which looked like
"Error: not registered" on the next place_call.

Fix:
- New TransportError::Deserialize(String) variant in wzp-proto
  carries serde errors as a distinct category.
- wzp-transport/reliable.rs::recv_signal returns Deserialize on
  serde_json::from_slice failures (was wrapped in Internal).
- wzp-relay/main.rs signal loop matches on Deserialize → warn +
  continue (instead of break).
- desktop/src-tauri/lib.rs recv loop does the same.

Other TransportError variants (ConnectionLost, Io, Internal) still
break the loop — only pure parse failures are recoverable.

This means future SignalMessage variant additions are backward-
compat by construction: older peers will see "unknown variant,
continuing" in their logs while newer peers can keep evolving the
protocol.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Siavash Sameni
2026-04-11 18:13:31 +04:00
parent 8cdf8d486a
commit da08723fe7
4 changed files with 37 additions and 2 deletions

View File

@@ -53,6 +53,15 @@ pub enum TransportError {
Timeout { ms: u64 },
#[error("io error: {0}")]
Io(#[from] std::io::Error),
/// Parsed wire bytes successfully but the payload didn't
/// deserialize into a known `SignalMessage` variant. Usually
/// means the peer is running a newer build with a variant we
/// don't know yet. Callers should **log and continue** rather
/// than tearing down the connection, so that forward-compat
/// additions to `SignalMessage` don't silently kill old
/// clients/relays.
#[error("signal deserialize: {0}")]
Deserialize(String),
#[error("internal transport error: {0}")]
Internal(String),
}

View File

@@ -1307,6 +1307,16 @@ async fn main() -> anyhow::Result<()> {
info!(%addr, "signal connection closed");
break;
}
Err(wzp_proto::TransportError::Deserialize(e)) => {
// Forward-compat: the peer sent a
// SignalMessage variant we don't know
// (newer client, newer federation peer).
// Log and continue — tearing down the
// connection on unknown variants would
// silently kill interop across minor
// protocol version bumps.
warn!(%addr, "signal deserialize (unknown variant?), continuing: {e}");
}
Err(e) => {
warn!(%addr, "signal recv error: {e}");
break;

View File

@@ -53,6 +53,13 @@ pub async fn recv_signal(recv: &mut quinn::RecvStream) -> Result<SignalMessage,
.await
.map_err(|e| TransportError::Internal(format!("stream read payload error: {e}")))?;
serde_json::from_slice(&payload)
.map_err(|e| TransportError::Internal(format!("signal deserialize error: {e}")))
serde_json::from_slice(&payload).map_err(|e| {
// Distinguish serde failures from transport failures so the
// caller (relay main loop, client recv loop) can continue on
// unknown-variant / parse errors instead of tearing down the
// whole signal connection. Forward-compat: adding a new
// `SignalMessage` variant in one side must not break the
// other side's signal connection.
TransportError::Deserialize(format!("{e}"))
})
}