rpc: fix trace result field written on exec error

.github/workflows/scripts/rpc_version.env

@@ -1,2 +1 @@
-
-RPC_VERSION=v2.20.0
+RPC_VERSION=v2.21.0

polygon/tracer/trace_bor_state_sync_txn.go

@@ -55,7 +55,6 @@ func TraceBorStateSyncTxnDebugAPI(
 	txCtx := initStateSyncTxContext(blockNum, blockHash)
 	tracer, streaming, cancel, err := transactions.AssembleTracer(ctx, traceConfig, txCtx.TxHash, uint256.NewInt(blockNum), blockHash, txIndex, stream, callTimeout)
 	if err != nil {
-		stream.WriteNil()
 		return gasUsed, err
 	}
 

rpc/handler.go

@@ -629,45 +629,6 @@ func (h *handler) handleSubscribe(cp *callProc, msg *jsonrpcMessage, stream json
 	return h.runMethod(ctx, msg, callb, args, stream)
 }
 
-// resultFieldStream lazily writes "result": on first value write.
-// This ensures JSON-RPC 2.0 compliance: if the method errors without
-// writing any data, "result" won't appear in the response.
-type resultFieldStream struct {
-	jsonstream.Stream
-	written bool
-}
-
-func (s *resultFieldStream) ensure() {
-	if !s.written {
-		s.written = true
-		s.Stream.WriteObjectField("result")
-	}
-}
-
-func (s *resultFieldStream) WriteNil()                   { s.ensure(); s.Stream.WriteNil() }
-func (s *resultFieldStream) WriteTrue()                  { s.ensure(); s.Stream.WriteTrue() }
-func (s *resultFieldStream) WriteFalse()                 { s.ensure(); s.Stream.WriteFalse() }
-func (s *resultFieldStream) WriteBool(val bool)          { s.ensure(); s.Stream.WriteBool(val) }
-func (s *resultFieldStream) WriteInt(val int)            { s.ensure(); s.Stream.WriteInt(val) }
-func (s *resultFieldStream) WriteInt8(val int8)          { s.ensure(); s.Stream.WriteInt8(val) }
-func (s *resultFieldStream) WriteInt16(val int16)        { s.ensure(); s.Stream.WriteInt16(val) }
-func (s *resultFieldStream) WriteInt32(val int32)        { s.ensure(); s.Stream.WriteInt32(val) }
-func (s *resultFieldStream) WriteInt64(val int64)        { s.ensure(); s.Stream.WriteInt64(val) }
-func (s *resultFieldStream) WriteUint(val uint)          { s.ensure(); s.Stream.WriteUint(val) }
-func (s *resultFieldStream) WriteUint8(val uint8)        { s.ensure(); s.Stream.WriteUint8(val) }
-func (s *resultFieldStream) WriteUint16(val uint16)      { s.ensure(); s.Stream.WriteUint16(val) }
-func (s *resultFieldStream) WriteUint32(val uint32)      { s.ensure(); s.Stream.WriteUint32(val) }
-func (s *resultFieldStream) WriteUint64(val uint64)      { s.ensure(); s.Stream.WriteUint64(val) }
-func (s *resultFieldStream) WriteFloat32(val float32)    { s.ensure(); s.Stream.WriteFloat32(val) }
-func (s *resultFieldStream) WriteFloat64(val float64)    { s.ensure(); s.Stream.WriteFloat64(val) }
-func (s *resultFieldStream) WriteString(val string)      { s.ensure(); s.Stream.WriteString(val) }
-func (s *resultFieldStream) WriteObjectStart()           { s.ensure(); s.Stream.WriteObjectStart() }
-func (s *resultFieldStream) WriteArrayStart()            { s.ensure(); s.Stream.WriteArrayStart() }
-func (s *resultFieldStream) WriteEmptyArray()            { s.ensure(); s.Stream.WriteEmptyArray() }
-func (s *resultFieldStream) WriteEmptyObject()           { s.ensure(); s.Stream.WriteEmptyObject() }
-func (s *resultFieldStream) Write(p []byte) (int, error) { s.ensure(); return s.Stream.Write(p) }
-func (s *resultFieldStream) WriteRaw(content string)     { s.ensure(); s.Stream.WriteRaw(content) }
-
 // remapDBOverload converts kv.ErrReadTxLimitExceeded into a JSON-RPC -32005 error and sets
 // the HTTP 503 flag in ctx so ServeHTTP can write the correct status before flushing.
 func remapDBOverload(ctx context.Context, err error) error {
@@ -702,12 +663,12 @@ func (h *handler) runMethod(ctx context.Context, msg *jsonrpcMessage, callb *cal
 		stream.Write(msg.ID)
 		stream.WriteMore()
 	}
-	rs := &resultFieldStream{Stream: stream}
+	rs := jsonstream.NewLazyFieldStream(stream, "result", false)
 	_, err := callb.call(ctx, msg.Method, args, rs)
 	if err != nil {
 		err = remapDBOverload(ctx, err)
-		if rs.written {
-			_ = stream.ClosePending(1) // the enclosing JSON object is explicitly handled below
+		if rs.Written() {
+			rs.CloseIfOpen()
 			stream.WriteMore()
 		}
 		HandleError(err, stream)

rpc/handler_test.go

@@ -55,6 +55,10 @@ func TestHandlerDoesNotDoubleWriteNull(t *testing.T) {
 			params:   []byte("[5]"),
 			expected: `{"jsonrpc":"2.0","id":1,"result":{"structLogs":{}},"error":{"code":-32000,"message":"id 4"}}`,
 		},
+		"err_with_unclosed_result_object": {
+			params:   []byte("[6]"),
+			expected: `{"jsonrpc":"2.0","id":1,"result":{"structLogs":[]},"error":{"code":-32000,"message":"id 6"}}`,
+		},
 	}
 
 	for name, testParams := range tests {
@@ -94,6 +98,14 @@ func TestHandlerDoesNotDoubleWriteNull(t *testing.T) {
 					stream.WriteObjectEnd()
 					return errors.New("id 4")
 				}
+				if id == 6 {
+					stream.WriteObjectStart()
+					stream.WriteObjectField("structLogs")
+					stream.WriteEmptyArray()
+					// intentionally leave the result object open: the tracer erroring out
+					// mid-write must not leave the response's "result" object unclosed.
+					return errors.New("id 6")
+				}
 				return nil
 			}
 

rpc/jsonrpc/debug_api_test.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math/big"
 	"reflect"
 	"testing"
 
@@ -250,6 +251,165 @@ func TestTraceErrorPathsWriteNoStream(t *testing.T) {
 		require.NoError(t, s.Flush())
 		require.Empty(t, buf.Bytes(), "stream must be empty on early error")
 	})
+
+	from := common.Address{0xFF}
+	to := common.Address{0x01}
+	gas := hexutil.Uint64(21000)
+	gasPrice := hexutil.Big(*big.NewInt(1e9))
+	traceCallArgs := ethapi.CallArgs{From: &from, To: &to, Gas: &gas, GasPrice: &gasPrice}
+	for _, tc := range []struct {
+		name   string
+		tracer string
+	}{
+		{"streaming", ""},
+		{"callTracer", "callTracer"},
+	} {
+		t.Run("TraceCall_execution_error_"+tc.name, func(t *testing.T) {
+			var cfg *tracersConfig.TraceConfig
+			if tc.tracer != "" {
+				name := tc.tracer
+				cfg = &tracersConfig.TraceConfig{Tracer: &name}
+			}
+			buf, s := newStream()
+			err := api.TraceCall(m.Ctx, traceCallArgs, rpc.BlockNumberOrHashWithNumber(1), cfg, s)
+			require.Error(t, err)
+			require.NoError(t, s.Flush())
+			require.Empty(t, buf.Bytes(), "stream must be empty on execution error so handler omits result field")
+		})
+	}
+
+	tracer := "callTracer"
+	timeout := "garbage"
+	badTimeoutCfg := &tracersConfig.TraceConfig{Tracer: &tracer, Timeout: &timeout}
+
+	t.Run("TraceTransaction_bad_timeout", func(t *testing.T) {
+		buf, s := newStream()
+		err := api.TraceTransaction(m.Ctx, common.HexToHash(debugTraceTransactionTests[0].txHash), badTimeoutCfg, s)
+		require.Error(t, err)
+		require.NoError(t, s.Flush())
+		require.Empty(t, buf.Bytes(), "stream must be empty on AssembleTracer error so handler omits result field")
+	})
+
+	t.Run("TraceCall_bad_timeout", func(t *testing.T) {
+		buf, s := newStream()
+		err := api.TraceCall(m.Ctx, traceCallArgs, rpc.BlockNumberOrHashWithNumber(1), badTimeoutCfg, s)
+		require.Error(t, err)
+		require.NoError(t, s.Flush())
+		require.Empty(t, buf.Bytes(), "stream must be empty on AssembleTracer error so handler omits result field")
+	})
+}
+
+// TestTxResultFieldStreamLazy verifies the lazy-write semantics of LazyFieldStream
+// with prependSeparator=true (the per-tx result field case).
+func TestTxResultFieldStreamLazy(t *testing.T) {
+	newInner := func() (*bytes.Buffer, jsonstream.Stream) {
+		var buf bytes.Buffer
+		return &buf, jsonstream.New(jsoniter.NewStream(jsoniter.ConfigDefault, &buf, 4096))
+	}
+
+	t.Run("no_writes_when_unused", func(t *testing.T) {
+		buf, inner := newInner()
+		_ = jsonstream.NewLazyFieldStream(inner, "result", true)
+		require.NoError(t, inner.Flush())
+		require.Empty(t, buf.Bytes())
+	})
+
+	t.Run("writes_separator_and_field_on_first_value", func(t *testing.T) {
+		buf, inner := newInner()
+		lazy := jsonstream.NewLazyFieldStream(inner, "result", true)
+		lazy.WriteNil()
+		require.NoError(t, inner.Flush())
+		require.Equal(t, `,"result":null`, buf.String())
+	})
+
+	t.Run("field_written_only_once", func(t *testing.T) {
+		buf, inner := newInner()
+		lazy := jsonstream.NewLazyFieldStream(inner, "result", true)
+		lazy.WriteArrayStart()
+		lazy.WriteString("a")
+		lazy.WriteMore()
+		lazy.WriteString("b")
+		lazy.WriteArrayEnd()
+		require.NoError(t, inner.Flush())
+		require.Equal(t, `,"result":["a","b"]`, buf.String())
+	})
+}
+
+// TestTraceBlockErrorBeforeWrite verifies traceBlock produces valid JSON when AssembleTracer fails
+// before any write (inner.Written stays false): each tx object has "error" but no "result" field.
+func TestTraceBlockErrorBeforeWrite(t *testing.T) {
+	m, _, _ := rpcdaemontest.CreateTestExecModule(t)
+	api := NewPrivateDebugAPI(newBaseApiForTest(m), m.DB, nil, 0, false)
+	ethApi := newEthApiForTest(newBaseApiForTest(m), m.DB, nil, nil)
+
+	tx, err := ethApi.GetTransactionByHash(m.Ctx, common.HexToHash(debugTraceTransactionTests[0].txHash))
+	require.NoError(t, err)
+	require.NotNil(t, tx)
+	blockNum := rpc.BlockNumber(tx.BlockNumber.ToInt().Uint64())
+
+	// Invalid timeout makes AssembleTracer fail before any write to inner, so inner.Written stays
+	// false and the error handler writes only the "error" field inside the tx object.
+	tracer := "callTracer"
+	timeout := "garbage"
+	cfg := &tracersConfig.TraceConfig{Tracer: &tracer, Timeout: &timeout}
+
+	var buf bytes.Buffer
+	s := jsonstream.NewStackStream(jsoniter.NewStream(jsoniter.ConfigDefault, &buf, 4096))
+	require.NoError(t, api.TraceBlockByNumber(m.Ctx, blockNum, cfg, s))
+	require.NoError(t, s.Flush())
+
+	var entries []json.RawMessage
+	require.NoError(t, json.Unmarshal(buf.Bytes(), &entries), "traceBlock output is not valid JSON: %s", buf.String())
+	require.NotEmpty(t, entries)
+	for i, entry := range entries {
+		var obj map[string]json.RawMessage
+		require.NoError(t, json.Unmarshal(entry, &obj), "tx entry %d is not a JSON object", i)
+		require.Contains(t, obj, "txHash", "tx entry %d missing txHash", i)
+		require.Contains(t, obj, "error", "tx entry %d: error must be inside the tx object, not at array level", i)
+		require.NotContains(t, obj, "result", "tx entry %d: result must not be written when error occurs before any write", i)
+	}
+}
+
+// TestTraceBlockErrorAfterWrite exercises the Written==true close path: when a tracer starts
+// writing to the result field before an error occurs, CloseIfOpen must seal the partial JSON
+// back to the tx-object level so the overall output remains valid.
+func TestTraceBlockErrorAfterWrite(t *testing.T) {
+	var buf bytes.Buffer
+	s := jsonstream.New(&buf)
+	inner := jsonstream.NewLazyFieldStream(s, "result", true)
+
+	// Replicate the per-tx structure of the traceBlock loop.
+	s.WriteArrayStart()
+	s.WriteObjectStart()
+	s.WriteObjectField("txHash")
+	s.WriteString("0xdeadbeef")
+	inner.ResetField()
+
+	// Simulate TraceTx writing a partial result before returning an error:
+	// the first write to inner triggers ensure() and sets Written=true.
+	inner.WriteObjectStart()
+	inner.WriteObjectField("from")
+	inner.WriteString("0xabcd")
+	// Replicate the traceBlock error handler.
+	inner.CloseIfOpen()
+	s.WriteMore()
+	s.WriteObjectField("error")
+	s.WriteString("partial write error")
+	s.WriteObjectEnd()
+
+	s.WriteArrayEnd()
+	require.NoError(t, s.Flush())
+
+	var entries []json.RawMessage
+	require.NoError(t, json.Unmarshal(buf.Bytes(), &entries), "traceBlock output is not valid JSON: %s", buf.String())
+	require.Len(t, entries, 1)
+	var obj map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal(entries[0], &obj), "tx entry is not a JSON object")
+	require.Contains(t, obj, "txHash")
+	require.Contains(t, obj, "result", "result must be present when Written=true before error")
+	require.Contains(t, obj, "error", "error must be inside the tx object, not at array level")
+	var resultObj map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal(obj["result"], &resultObj), "result must be valid JSON after CloseIfOpen: %s", obj["result"])
 }
 
 func TestTraceTransactionNoRefund(t *testing.T) {

rpc/jsonrpc/tracing.go

@@ -149,6 +149,7 @@ func (api *DebugAPIImpl) traceBlock(ctx context.Context, blockNrOrHash rpc.Block
 	}
 
 	var gasUsed uint64
+	inner := jsonstream.NewLazyFieldStream(stream, "result", true)
 	for txnIndex, txn := range txns {
 		isBorStateSyncTxn := borStateSyncTxn == txn
 		var txnHash common.Hash
@@ -161,15 +162,15 @@ func (api *DebugAPIImpl) traceBlock(ctx context.Context, blockNrOrHash rpc.Block
 		stream.WriteObjectStart()
 		stream.WriteObjectField("txHash")
 		stream.WriteString(txnHash.Hex())
-		stream.WriteMore()
-		stream.WriteObjectField("result")
 		select {
 		default:
 		case <-ctx.Done():
 			return ctx.Err()
 		}
 		ibs.SetTxContext(blockCtx.BlockNumber, txnIndex)
 
+		inner.ResetField()
+
 		if isBorStateSyncTxn {
 			var stateSyncEvents []*types.Message
 			stateSyncEvents, err = api.bridgeReader.Events(ctx, block.Hash(), blockNumber)
@@ -187,7 +188,7 @@ func (api *DebugAPIImpl) traceBlock(ctx context.Context, blockNrOrHash rpc.Block
 				block.NumberU64(),
 				block.Time(),
 				blockCtx,
-				stream,
+				inner,
 				api.evmCallTimeout,
 				stateSyncEvents,
 				txnIndex,
@@ -196,8 +197,6 @@ func (api *DebugAPIImpl) traceBlock(ctx context.Context, blockNrOrHash rpc.Block
 		} else {
 			msg, asMessageErr := txn.AsMessage(*signer, block.BaseFee(), rules)
 			if asMessageErr != nil {
-				// Fail closed here because tracing needs a valid Message-derived sender/fee context.
-				stream.WriteNil()
 				err = fmt.Errorf("convert transaction %s to message: %w", txnHash, asMessageErr)
 			} else {
 				txCtx := evmtypes.TxContext{
@@ -208,16 +207,16 @@ func (api *DebugAPIImpl) traceBlock(ctx context.Context, blockNrOrHash rpc.Block
 				}
 
 				var _gasUsed uint64
-				_gasUsed, err = transactions.TraceTx(ctx, engine, txn, msg, blockCtx, txCtx, &block.Header().Number, block.Hash(), txnIndex, ibs, config, chainConfig, stream, api.evmCallTimeout, precompiles)
+				_gasUsed, err = transactions.TraceTx(ctx, engine, txn, msg, blockCtx, txCtx, &block.Header().Number, block.Hash(), txnIndex, ibs, config, chainConfig, inner, api.evmCallTimeout, precompiles)
 				gasUsed += _gasUsed
 			}
 		}
 		if err == nil {
 			err = ibs.FinalizeTx(rules, state.NewNoopWriter())
 		}
 
-		// if we have an error we want to output valid json for it before continuing after clearing down potential writes to the stream
 		if err != nil {
+			inner.CloseIfOpen()
 			stream.WriteMore()
 			rpc.HandleError(err, stream)
 		}

rpc/jsonstream/iterator_stream.go

@@ -169,6 +169,8 @@ func (s *JsoniterStream) Size() int {
 }
 
 // ClosePending does nothing because JsoniterStream wrapper does not support closing pending elements
-func (s *JsoniterStream) ClosePending( /*skipLast*/ uint) error {
+func (s *JsoniterStream) ClosePending( /*targetDepth*/ uint) error {
 	return nil
 }
+
+func (s *JsoniterStream) Depth() int { return 0 }