Repository: google/go-github (go)

Issue:
UploadSarif Always Returns An Error
In version 57.0.0 of go-github, [UploadSarif](https://github.com/google/go-github/blob/v57.0.0/github/code-scanning.go#L384) never returns a valid SarifID object because it's propagating the AcceptedError instead of deserializing the body because [this endpoint](https://docs.github.com/en/rest/code-scanning/code-scanning?apiVersion=2022-11-28#upload-an-analysis-as-sarif-data) returns a 202. It seems to exhibit the same behavior that was fixed for CreateFork in #878.

Pull request description:
Fix issue in AcceptedError handling for UploadSarif
Fixes #3036 

- Update successful test case for UploadSarif to validate the expected return data
- Handle the AcceptedError in UploadSarif and unmarshal a SarifID from the error's Raw attribute

Patch:
diff --git a/github/code-scanning.go b/github/code-scanning.go
index 74a7b6c9..a8fca98a 100644
--- a/github/code-scanning.go
+++ b/github/code-scanning.go
@@ -7,6 +7,8 @@ package github
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -389,11 +391,21 @@ func (s *CodeScanningService) UploadSarif(ctx context.Context, owner, repo strin
 		return nil, nil, err
 	}
 
-	sarifID := new(SarifID)
-	resp, err := s.client.Do(ctx, req, sarifID)
-	if err != nil {
+	// This will always return an error without unmarshalling the data
+	resp, err := s.client.Do(ctx, req, nil)
+	// Even though there was an error, we still return the response
+	// in case the caller wants to inspect it further.
+	// However, if the error is AcceptedError, decode it below before
+	// returning from this function and closing the response body.
+	var acceptedError *AcceptedError
+	if !errors.As(err, &acceptedError) {
 		return nil, resp, err
 	}
+	sarifID := new(SarifID)
+	decErr := json.Unmarshal(acceptedError.Raw, sarifID)
+	if decErr != nil {
+		return nil, resp, decErr
+	}
 
 	return sarifID, resp, nil
 }


Test patch:
diff --git a/github/code-scanning_test.go b/github/code-scanning_test.go
index 4fbe45e7..a081a6ba 100644
--- a/github/code-scanning_test.go
+++ b/github/code-scanning_test.go
@@ -58,6 +58,11 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
+	expectedSarifID := &SarifID{
+		ID:  String("testid"),
+		URL: String("https://example.com/testurl"),
+	}
+
 	mux.HandleFunc("/repos/o/r/code-scanning/sarifs", func(w http.ResponseWriter, r *http.Request) {
 		v := new(SarifAnalysis)
 		assertNilError(t, json.NewDecoder(r.Body).Decode(v))
@@ -67,15 +72,20 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
 			t.Errorf("Request body = %+v, want %+v", v, want)
 		}
 
-		fmt.Fprint(w, `{"commit_sha":"abc","ref":"ref/head/main","sarif":"abc"}`)
+		w.WriteHeader(http.StatusAccepted)
+		respBody, _ := json.Marshal(expectedSarifID)
+		_, _ = w.Write(respBody)
 	})
 
 	ctx := context.Background()
 	sarifAnalysis := &SarifAnalysis{CommitSHA: String("abc"), Ref: String("ref/head/main"), Sarif: String("abc"), CheckoutURI: String("uri"), StartedAt: &Timestamp{time.Date(2006, time.January, 02, 15, 04, 05, 0, time.UTC)}, ToolName: String("codeql-cli")}
-	_, _, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
+	respSarifID, _, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
 	if err != nil {
 		t.Errorf("CodeScanning.UploadSarif returned error: %v", err)
 	}
+	if !cmp.Equal(expectedSarifID, respSarifID) {
+		t.Errorf("Sarif response = %+v, want %+v", respSarifID, expectedSarifID)
+	}
 
 	const methodName = "UploadSarif"
 	testBadOptions(t, methodName, func() (err error) {