From d27f9cab684eac8fa4fcca2df02448687f1efd92 Mon Sep 17 00:00:00 2001 From: chatton Date: Thu, 22 Jan 2026 10:26:01 +0000 Subject: [PATCH] fix: return values correctly not nil --- block/internal/executing/tracing.go | 6 +- block/internal/executing/tracing_test.go | 97 ++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/block/internal/executing/tracing.go b/block/internal/executing/tracing.go index fe5fae0ed..137d9170b 100644 --- a/block/internal/executing/tracing.go +++ b/block/internal/executing/tracing.go @@ -48,7 +48,7 @@ func (t *tracedBlockProducer) RetrieveBatch(ctx context.Context) (*BatchData, er if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return nil, err + return batchData, err } if batchData != nil && batchData.Batch != nil { @@ -77,7 +77,7 @@ func (t *tracedBlockProducer) CreateBlock(ctx context.Context, height uint64, ba if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return nil, nil, err + return header, data, err } return header, data, nil } @@ -95,7 +95,7 @@ func (t *tracedBlockProducer) ApplyBlock(ctx context.Context, header types.Heade if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return types.State{}, err + return state, err } span.SetAttributes( diff --git a/block/internal/executing/tracing_test.go b/block/internal/executing/tracing_test.go index fa5f4bebc..396e1d8a4 100644 --- a/block/internal/executing/tracing_test.go +++ b/block/internal/executing/tracing_test.go @@ -320,3 +320,100 @@ func TestTracedBlockProducer_ValidateBlock_Error(t *testing.T) { require.Equal(t, codes.Error, span.Status().Code) require.Equal(t, "validation failed", span.Status().Description) } + +// TestTracedBlockProducer_RetrieveBatch_ErrorWithValue verifies that when the inner +// function returns both a value and an error, the value is passed through (not nil). +// this is important for cases like ErrNoTransactionsInBatch where valid data accompanies the error. +func TestTracedBlockProducer_RetrieveBatch_ErrorWithValue(t *testing.T) { + expectedBatch := &BatchData{ + Batch: &coresequencer.Batch{ + Transactions: [][]byte{}, + }, + } + mock := &mockBlockProducer{ + retrieveBatchFn: func(ctx context.Context) (*BatchData, error) { + return expectedBatch, errors.New("no transactions in batch") + }, + } + producer, sr := setupBlockProducerTrace(t, mock) + ctx := context.Background() + + batch, err := producer.RetrieveBatch(ctx) + require.Error(t, err) + require.Equal(t, "no transactions in batch", err.Error()) + require.NotNil(t, batch, "batch should not be nil when inner returns value with error") + require.Same(t, expectedBatch, batch, "batch should be the same instance returned by inner") + + spans := sr.Ended() + require.Len(t, spans, 1) + span := spans[0] + require.Equal(t, codes.Error, span.Status().Code) +} + +// TestTracedBlockProducer_CreateBlock_ErrorWithValue verifies that when the inner +// function returns both values and an error, the values are passed through (not nil). +func TestTracedBlockProducer_CreateBlock_ErrorWithValue(t *testing.T) { + expectedHeader := &types.SignedHeader{ + Header: types.Header{ + BaseHeader: types.BaseHeader{ + Height: 100, + }, + }, + } + expectedData := &types.Data{ + Txs: types.Txs{[]byte("tx1")}, + } + mock := &mockBlockProducer{ + createBlockFn: func(ctx context.Context, height uint64, batchData *BatchData) (*types.SignedHeader, *types.Data, error) { + return expectedHeader, expectedData, errors.New("partial failure") + }, + } + producer, sr := setupBlockProducerTrace(t, mock) + ctx := context.Background() + + header, data, err := producer.CreateBlock(ctx, 100, nil) + require.Error(t, err) + require.Equal(t, "partial failure", err.Error()) + require.NotNil(t, header, "header should not be nil when inner returns value with error") + require.NotNil(t, data, "data should not be nil when inner returns value with error") + require.Same(t, expectedHeader, header, "header should be the same instance returned by inner") + require.Same(t, expectedData, data, "data should be the same instance returned by inner") + + spans := sr.Ended() + require.Len(t, spans, 1) + span := spans[0] + require.Equal(t, codes.Error, span.Status().Code) +} + +// TestTracedBlockProducer_ApplyBlock_ErrorWithValue verifies that when the inner +// function returns both a state and an error, the state is passed through (not zero value). +func TestTracedBlockProducer_ApplyBlock_ErrorWithValue(t *testing.T) { + expectedState := types.State{ + AppHash: []byte{0xde, 0xad, 0xbe, 0xef}, + LastBlockHeight: 50, + } + mock := &mockBlockProducer{ + applyBlockFn: func(ctx context.Context, header types.Header, data *types.Data) (types.State, error) { + return expectedState, errors.New("partial apply failure") + }, + } + producer, sr := setupBlockProducerTrace(t, mock) + ctx := context.Background() + + header := types.Header{ + BaseHeader: types.BaseHeader{ + Height: 50, + }, + } + + state, err := producer.ApplyBlock(ctx, header, &types.Data{}) + require.Error(t, err) + require.Equal(t, "partial apply failure", err.Error()) + require.Equal(t, expectedState.AppHash, state.AppHash, "state should preserve AppHash when inner returns value with error") + require.Equal(t, expectedState.LastBlockHeight, state.LastBlockHeight, "state should preserve LastBlockHeight when inner returns value with error") + + spans := sr.Ended() + require.Len(t, spans, 1) + span := spans[0] + require.Equal(t, codes.Error, span.Status().Code) +}