mirror of
https://github.com/luxfi/api.git
synced 2026-09-24 00:24:53 +00:00
fix(zap): correctness — distinct MsgErrorFlag, response flag on success, drop debug stderr
Three real bugs in luxfi/api/zap: 1. Debug fmt.Fprintf(os.Stderr, ...) prints on every wire read/write/ handler invocation. Visible in production logs as [ZAP-WIRE-READ], [ZAP-CLIENT-READ], [ZAP-SERVER-WRITE], [ZAP-HANDLER]. Stripped. 2. Server's success path sent the handler's returned msgType directly without OR'ing MsgResponseFlag. Echo-style handlers (return same msgType) produced responses that looked like fresh inbound requests to the client demultiplexer, which dropped them. Tests caught this as 'Expected response type 137, got 9'. 3. Client treated every response as an error because the protocol used a single MsgResponseFlag for both success and error paths. Added MsgErrorFlag (0x40) — error responses now carry MsgResponseFlag AND MsgErrorFlag; success responses carry only MsgResponseFlag. All msg types must stay < 0x40 (the existing range 1-52 is fine).
This commit is contained in:
+11
-17
@@ -11,7 +11,6 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"os"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
@@ -207,9 +206,6 @@ func (c *Conn) readLoop() {
|
||||
return
|
||||
}
|
||||
|
||||
fmt.Fprintf(os.Stderr, "[ZAP-CLIENT-READ] msgType=0x%02x payloadLen=%d responseFlag=%v\n",
|
||||
byte(msgType), len(payload), msgType&MsgResponseFlag != 0)
|
||||
|
||||
// Extract request ID from response
|
||||
if len(payload) < 4 {
|
||||
continue
|
||||
@@ -222,11 +218,12 @@ func (c *Conn) readLoop() {
|
||||
c.mu.Unlock()
|
||||
|
||||
if ok {
|
||||
// Check for error response (MsgResponseFlag set)
|
||||
if msgType&MsgResponseFlag != 0 {
|
||||
errMsg := string(respPayload)
|
||||
// Error responses carry MsgErrorFlag and have the error
|
||||
// string as their payload. Success responses carry only
|
||||
// MsgResponseFlag and have the application payload.
|
||||
if msgType&MsgErrorFlag != 0 {
|
||||
ch <- &response{
|
||||
err: fmt.Errorf("remote error: %s", errMsg),
|
||||
err: fmt.Errorf("remote error: %s", string(respPayload)),
|
||||
}
|
||||
} else {
|
||||
ch <- &response{
|
||||
@@ -343,9 +340,6 @@ func (c *ServerConn) Write(reqID uint32, msgType MessageType, payload []byte) er
|
||||
c.writeMu.Lock()
|
||||
defer c.writeMu.Unlock()
|
||||
|
||||
fmt.Fprintf(os.Stderr, "[ZAP-SERVER-WRITE] msgType=0x%02x reqID=%d payloadLen=%d responseFlag=%v\n",
|
||||
byte(msgType), reqID, len(payload), msgType&MsgResponseFlag != 0)
|
||||
|
||||
if c.config.WriteTimeout > 0 {
|
||||
_ = c.conn.SetWriteDeadline(time.Now().Add(c.config.WriteTimeout))
|
||||
}
|
||||
@@ -461,21 +455,21 @@ func (s *Server) handleConn(ctx context.Context, conn *ServerConn) {
|
||||
go func(reqID uint32, msgType MessageType, payload []byte) {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
// Send panic as error response
|
||||
errMsg := fmt.Sprintf("handler panic: %v", r)
|
||||
conn.Write(reqID, msgType|MsgResponseFlag, []byte(errMsg))
|
||||
conn.Write(reqID, msgType|MsgResponseFlag|MsgErrorFlag, []byte(errMsg))
|
||||
}
|
||||
}()
|
||||
|
||||
respType, respPayload, err := s.handler.Handle(ctx, msgType, payload)
|
||||
fmt.Fprintf(os.Stderr, "[ZAP-HANDLER] reqMsgType=0x%02x respType=0x%02x err=%v respLen=%d\n",
|
||||
byte(msgType), byte(respType), err, len(respPayload))
|
||||
if err != nil {
|
||||
conn.Write(reqID, msgType|MsgResponseFlag, []byte(err.Error()))
|
||||
conn.Write(reqID, msgType|MsgResponseFlag|MsgErrorFlag, []byte(err.Error()))
|
||||
return
|
||||
}
|
||||
|
||||
conn.Write(reqID, respType, respPayload)
|
||||
// Success responses carry only MsgResponseFlag — the client
|
||||
// demultiplexer routes by request ID and distinguishes
|
||||
// success from error via MsgErrorFlag.
|
||||
conn.Write(reqID, respType|MsgResponseFlag, respPayload)
|
||||
}(reqID, msgType, payload)
|
||||
}
|
||||
}
|
||||
|
||||
+12
-8
@@ -6,9 +6,7 @@ package zap
|
||||
import (
|
||||
"encoding/binary"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"sync"
|
||||
)
|
||||
|
||||
@@ -77,9 +75,18 @@ const (
|
||||
MsgWarpGetPublicKey MessageType = 51
|
||||
MsgWarpBatchSign MessageType = 52
|
||||
|
||||
// Response flag - set on response messages (high bit)
|
||||
// All message types must be < 128 to allow OR with this flag
|
||||
MsgResponseFlag MessageType = 128
|
||||
// Response flag - set on response messages (bit 7 / high bit).
|
||||
// All message types must be < 64 (0x40) to allow OR with this flag
|
||||
// AND with MsgErrorFlag below.
|
||||
MsgResponseFlag MessageType = 0x80
|
||||
|
||||
// Error flag - set on response messages that carry an error.
|
||||
// Always co-occurs with MsgResponseFlag on the wire:
|
||||
// success response: msgType | MsgResponseFlag
|
||||
// error response: msgType | MsgResponseFlag | MsgErrorFlag
|
||||
// Payload of an error response is the error string (length-prefixed
|
||||
// after the request ID).
|
||||
MsgErrorFlag MessageType = 0x40
|
||||
)
|
||||
|
||||
// Buffer is a reusable byte buffer for zero-copy operations
|
||||
@@ -326,9 +333,6 @@ func ReadMessage(r io.Reader) (MessageType, []byte, error) {
|
||||
length := binary.BigEndian.Uint32(header[0:4])
|
||||
msgType := MessageType(header[4])
|
||||
|
||||
fmt.Fprintf(os.Stderr, "[ZAP-WIRE-READ] header=%x len=%d msgType=0x%02x\n",
|
||||
header, length, byte(msgType))
|
||||
|
||||
if length > MaxMessageSize {
|
||||
return 0, nil, ErrMessageTooLarge
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user