P2P: message.Send checks the request's ErrMessage instead of the response's — receiver-side processing errors are silently acked as success

[jackzampolin]

Summary

message.Send inspects m.GetErrMessage() — the outbound request, whose ErrMessage is never set — instead of respMessage.GetErrMessage(), the reply actually carrying the receiver's error:

defradb/internal/db/p2p/message/message.go

Lines 173 to 183 in 68eafac

	select {
	case respMessage := <-responseChan:
	if m.GetErrMessage() != "" {
	return resp, errors.New(respMessage.GetErrMessage())
	}
	switch typedResp := respMessage.(type) {
	case ResponseType:
	return typedResp, nil
	default:
	return resp, NewErrResponseType(resp, typedResp)
	}

	case respMessage := <-responseChan:
		if m.GetErrMessage() != "" {                     // ← checks the request, always empty
			return resp, errors.New(respMessage.GetErrMessage())
		}

The receiver dutifully sets the error on the response (comm_channel.go onRequest → respPtr.SetErrMessage(err.Error()), comm_channel.go#L98-L127), but the sender never reads it. Verified on develop @ 68eafac2 (also present in v1.0.0).

Impact

Every replicator push whose processing fails on the receiver — any merge error: index constraint, ACP, corrupt block, anything processPushlogRequest returns — is treated as success by the sender:

• no retry record is created (handleReplicatorFailure is never reached from the error-reply path);
• during a scheduled retry, the doc's retry record is deleted as if delivered (replicator.go retryDoc success path);
• the replicator is reported Active.

Net effect: the 30s→32m retry ladder (node.go#L111-L119) only functions for transport failures (timeout/no reply). For receiver-side processing failures the sender is blind — silent, permanent divergence with the replicator status showing healthy. Transport-level failures still retry correctly, which is why this is easy to miss.

Reproduction

Passing characterization test (two nodes, unique index, colliding value):
TestIndexP2P_UniqueConflictIsDroppedByReplicatorRetryQueue in jackzampolin/defradb@8b961abe (tests/integration/index/index_p2p_test.go). It shows: the receiver rejects and rolls back the merge, yet the sender's retry records disappear, a later doc in the same batch delivers fine, and the replicator returns to Active — the rejected doc is simply gone from the sender's obligations.

Fix

One token: m.GetErrMessage() → respMessage.GetErrMessage() at message.go#L175.

Heads-up on a coupled consequence: once error replies propagate, a deterministic receiver-side failure (e.g. a unique-index conflict) changes from silently-dropped to retried-forever — and retryReplicator aborts a peer's whole retry batch on the first failing doc (replicator.go#L722-L786), so one permanently-failing doc would head-of-line-block every other doc queued to that peer. That deserves its own decision — filed separately (see the companion issue on unique-index/CRDT-merge convergence, linked below once filed).

Found while building the Rust port's go-compat parity suite (sourcenetwork/defradb.rs#1134).

[jackzampolin]

Companion issue for the coupled deterministic-failure consequence: #5059 (unique-index × CRDT-merge convergence).

[jsimnz]

AFAICT this is a bug/typo on the err check. Ez fix as youve outlined