fix(metrics): fallback to mock writer on file open failure

When timestamps are enabled but the target file cannot be opened,
NewZerologMetrics returned nil. This caused a nil pointer dereference
on subsequent metrics.Capture() calls.

This fix gracefully degrades to a no-op mockWriter and logs a warning.
Adds a regression test to verify the fallback behavior.

PR: #596
Fixes: #595
Signed-off-by: abhaygoudannavar <abhaysgoudnvr@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

Author: abhaygoudannavar

cmd/urunc/main.go

@@ -140,7 +140,10 @@ func main() {
 			if err != nil {
 				return nil, err
 			}
-			metrics = m.NewZerologMetrics(cfg.Timestamps.Enabled, cfg.Timestamps.Destination, "")
+			metrics, err = m.NewZerologMetrics(cfg.Timestamps.Enabled, cfg.Timestamps.Destination, "")
+			if err != nil {
+				logrus.Warnf("Metrics will be disabled: %v", err)
+			}
 			return nil, nil
 		},
 	}

internal/metrics/metrics.go

@@ -15,6 +15,7 @@
 package metrics
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/rs/zerolog"
@@ -51,21 +52,23 @@ func (z *zerologMetrics) SetLoggerContainerID(containerID string) {
 	z.containerID = containerID
 }
 
-// NewZerologMetrics creates a Writer that logs timestamps for a single container
-func NewZerologMetrics(enabled bool, target string, containerID string) Writer {
+// NewZerologMetrics creates a Writer that logs timestamps for a single container.
+// On file open failure it returns a no-op mockWriter and an error, allowing the
+// caller to log or handle the error as appropriate.
+func NewZerologMetrics(enabled bool, target string, containerID string) (Writer, error) {
 	if enabled {
 		zerolog.TimeFieldFormat = zerolog.TimeFormatUnixNano
 		file, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
-			return nil
+			return &mockWriter{}, fmt.Errorf("failed to open metrics file %s: %w", target, err)
 		}
 		logger := zerolog.New(file).Level(zerolog.InfoLevel).With().Timestamp().Logger()
 		return &zerologMetrics{
 			logger:      &logger,
 			containerID: containerID,
-		}
+		}, nil
 	}
-	return &mockWriter{}
+	return &mockWriter{}, nil
 }
 
 type mockWriter struct{}

internal/metrics/metrics_test.go

@@ -20,6 +20,8 @@ import (
 	"testing"
 
 	"github.com/rs/zerolog"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestZerologMetricsMetadata(t *testing.T) {
@@ -35,20 +37,25 @@ func TestZerologMetricsMetadata(t *testing.T) {
 
 	line := buf.String()
 	var m map[string]any
-	if err := json.Unmarshal([]byte(line), &m); err != nil {
-		t.Fatalf("failed to parse log: %v", err)
-	}
+	err := json.Unmarshal([]byte(line), &m)
+	require.NoError(t, err, "failed to parse log output")
 
-	if got := m["containerID"]; got != "container00" {
-		t.Errorf("containerID = %v, want container00", got)
-	}
-	if got := m["timestampID"]; got != "TS00" {
-		t.Errorf("timestampID = %v, want TS00", got)
-	}
-	if got := m["timestampName"]; got != "CR.invoked" {
-		t.Errorf("timestampName = %v, want CR.invoked", got)
-	}
-	if got := int(m["timestampOrder"].(float64)); got != 0 {
-		t.Errorf("timestampOrder = %v, want 0", got)
-	}
+	assert.Equal(t, "container00", m["containerID"], "containerID mismatch")
+	assert.Equal(t, "TS00", m["timestampID"], "timestampID mismatch")
+	assert.Equal(t, "CR.invoked", m["timestampName"], "timestampName mismatch")
+	assert.Equal(t, float64(0), m["timestampOrder"], "timestampOrder mismatch")
+}
+
+func TestZerologMetricsInvalidFileDoesNotPanic(t *testing.T) {
+	// Provide a path to a directory that definitely does not exist
+	invalidPath := "/does/not/exist/timestamps.log"
+
+	// Create the metrics writer with timestamps enabled but an invalid path
+	writer, err := NewZerologMetrics(true, invalidPath, "container-test")
+
+	require.Error(t, err, "expected an error for invalid file path")
+	require.NotNil(t, writer, "expected non-nil writer (fallback to mockWriter), got nil")
+
+	_, isMock := writer.(*mockWriter)
+	assert.True(t, isMock, "expected writer to be *mockWriter on file open failure")
 }