fix: return values correctly not nil (#3004)

Author: chatton

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(

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)
+}