From fbc5fd8cab826ee42a437b0cdcf003ca7bfdf250 Mon Sep 17 00:00:00 2001
From: Zaki Shaikh <zashaikh@redhat.com>
Date: Tue, 4 Nov 2025 16:10:47 +0530
Subject: [PATCH] enhance: add PipelineRun name in pr json marshalling

when a PipelineRun fails json marshalling in match.go
we don't know which PipelineRun has syntax error if
there are multiple PipelineRuns. this PR adds
PipelineRun name along with Json marshalling error so
that it is easy to track the issue.

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
---
 pkg/pipelineascode/match.go      |  9 +++-
 pkg/pipelineascode/match_test.go | 71 ++++++++++++++++++++++++++------
 2 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go
index a6a61984ad..f4886c764d 100644
--- a/pkg/pipelineascode/match.go
+++ b/pkg/pipelineascode/match.go
@@ -396,9 +396,14 @@ func filterRunningPipelineRunOnTargetTest(testPipeline string, prs []*tektonv1.P
 // - the template variable with the one from the event (this includes the remote pipeline that has template variables).
 func (p *PacRun) changePipelineRun(ctx context.Context, repo *v1alpha1.Repository, prs []*tektonv1.PipelineRun) error {
 	for k, pr := range prs {
+		prName := pr.GetName()
+		if prName == "" {
+			prName = pr.GetGenerateName()
+		}
+
 		b, err := json.Marshal(pr)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to marshal PipelineRun %s: %w", prName, err)
 		}
 
 		name := secrets.GenerateBasicAuthSecretName()
@@ -410,7 +415,7 @@ func (p *PacRun) changePipelineRun(ctx context.Context, repo *v1alpha1.Repositor
 		var np *tektonv1.PipelineRun
 		err = json.Unmarshal([]byte(processed), &np)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to unmarshal PipelineRun %s: %w", prName, err)
 		}
 		// don't crash when we don't have any annotations
 		if np.Annotations == nil {
diff --git a/pkg/pipelineascode/match_test.go b/pkg/pipelineascode/match_test.go
index d95358f2e9..30ab712799 100644
--- a/pkg/pipelineascode/match_test.go
+++ b/pkg/pipelineascode/match_test.go
@@ -58,6 +58,13 @@ func TestPacRun_checkNeedUpdate(t *testing.T) {
 }
 
 func TestChangePipelineRun(t *testing.T) {
+	repo := &v1alpha1.Repository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "testrepo",
+			Namespace: "test",
+		},
+	}
+
 	prs := []*tektonv1.PipelineRun{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -66,21 +73,59 @@ func TestChangePipelineRun(t *testing.T) {
 			},
 		},
 	}
-	event := info.NewEvent()
-	event.Repository = "testrepo"
-	p := &PacRun{event: event}
-	repo := &v1alpha1.Repository{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "testrepo",
-			Namespace: "test",
+
+	jsonErrorPRs := []*tektonv1.PipelineRun{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "json-error-pr",
+				Namespace: "test",
+			},
+			Spec: tektonv1.PipelineRunSpec{
+				Params: []tektonv1.Param{
+					{
+						Name: "my-param",
+						Value: tektonv1.ParamValue{
+							// We are intentionally leaving this empty:
+							// Type: tektonv1.ParamTypeString,
+							StringVal: "some-value",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	tests := []struct {
+		name          string
+		prs           []*tektonv1.PipelineRun
+		expectedError string
+	}{
+		{
+			name: "test with params",
+			prs:  prs,
 		},
+		{
+			name:          "test with json error",
+			prs:           jsonErrorPRs,
+			expectedError: "failed to marshal PipelineRun json-error-pr: json: error calling MarshalJSON for type v1.ParamValue: impossible ParamValues.Type: \"\"",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			event := info.NewEvent()
+			event.Repository = "testrepo"
+			p := &PacRun{event: event}
+			ctx, _ := rtesting.SetupFakeContext(t)
+			err := p.changePipelineRun(ctx, repo, tt.prs)
+			if tt.expectedError != "" {
+				assert.Error(t, err, tt.expectedError)
+				return
+			}
+			assert.Assert(t, strings.HasPrefix(tt.prs[0].GetName(), "pac-gitauth"), tt.prs[0].GetName(), "has no pac-gitauth prefix")
+			assert.Assert(t, tt.prs[0].GetAnnotations()[apipac.GitAuthSecret] != "")
+			assert.Assert(t, tt.prs[0].GetNamespace() == "testrepo", "namespace should be testrepo: %v", tt.prs[0].GetNamespace())
+		})
 	}
-	ctx, _ := rtesting.SetupFakeContext(t)
-	err := p.changePipelineRun(ctx, repo, prs)
-	assert.NilError(t, err)
-	assert.Assert(t, strings.HasPrefix(prs[0].GetName(), "pac-gitauth"), prs[0].GetName(), "has no pac-gitauth prefix")
-	assert.Assert(t, prs[0].GetAnnotations()[apipac.GitAuthSecret] != "")
-	assert.Assert(t, prs[0].GetNamespace() == "testrepo", "namespace should be testrepo: %v", prs[0].GetNamespace())
 }
 
 func TestFilterRunningPipelineRunOnTargetTest(t *testing.T) {