go-concourse: configs: JSON parsing: improve error message given to t…

…he end user Signed-off-by: aliculPix4D <[email protected]>
Pix4D · Oct 4, 2024 · 3cf91e9 · 3cf91e9
1 parent a8c6204
commit 3cf91e9
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 11 deletions.
diff --git a/go-concourse/concourse/configs.go b/go-concourse/concourse/configs.go
@@ -3,6 +3,7 @@ package concourse
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -85,20 +86,36 @@ func (team *team) CreateOrUpdatePipelineConfig(pipelineRef atc.PipelineRef, conf
 
 	switch response.StatusCode {
 	case http.StatusOK, http.StatusCreated:
-		configResponse := setConfigResponse{}
-		err = json.Unmarshal(body, &configResponse)
-		if err != nil {
-			return false, false, []ConfigWarning{}, err
+		if response.Header.Get("Content-Type") == "application/json" {
+			configResponse := setConfigResponse{}
+			err = json.Unmarshal(body, &configResponse)
+			if err != nil {
+				return false, false, []ConfigWarning{}, fmt.Errorf("client: unable to JSON parse the response body: %w", err)
+			}
+			created := response.StatusCode == http.StatusCreated
+			return created, !created, configResponse.Warnings, nil
+		} else {
+			return false, false, []ConfigWarning{}, internal.UnexpectedResponseError{
+				StatusCode: response.StatusCode,
+				Status:     response.Status,
+				Body:       string(body),
+			}
 		}
-		created := response.StatusCode == http.StatusCreated
-		return created, !created, configResponse.Warnings, nil
 	case http.StatusBadRequest:
-		var validationErr atc.SaveConfigResponse
-		err = json.Unmarshal(body, &validationErr)
-		if err != nil {
-			return false, false, []ConfigWarning{}, err
+		if response.Header.Get("Content-Type") == "application/json" {
+			var validationErr atc.SaveConfigResponse
+			err = json.Unmarshal(body, &validationErr)
+			if err != nil {
+				return false, false, []ConfigWarning{}, fmt.Errorf("client: unable to JSON parse the response body: %w", err)
+			}
+			return false, false, []ConfigWarning{}, InvalidConfigError{Errors: validationErr.Errors}
+		} else {
+			return false, false, []ConfigWarning{}, internal.UnexpectedResponseError{
+				StatusCode: response.StatusCode,
+				Status:     response.Status,
+				Body:       string(body),
+			}
 		}
-		return false, false, []ConfigWarning{}, InvalidConfigError{Errors: validationErr.Errors}
 	case http.StatusForbidden:
 		return false, false, []ConfigWarning{}, internal.ForbiddenError{
 			Reason: string(body),

diff --git a/go-concourse/concourse/configs_test.go b/go-concourse/concourse/configs_test.go
@@ -175,6 +175,7 @@ var _ = Describe("ATC Handler Configs", func() {
 			expectedVersion string
 			expectedConfig  []byte
 
+			contentType  string
 			returnHeader int
 			returnBody   []byte
 
@@ -204,6 +205,10 @@ var _ = Describe("ATC Handler Configs", func() {
 
 						Expect(receivedConfig).To(Equal(expectedConfig))
 
+						if contentType != "" {
+							w.Header().Set("Content-Type", contentType)
+						}
+
 						w.WriteHeader(returnHeader)
 						w.Write(returnBody)
 					},
@@ -213,6 +218,7 @@ var _ = Describe("ATC Handler Configs", func() {
 
 		Context("when creating a new config", func() {
 			BeforeEach(func() {
+				contentType = "application/json"
 				returnHeader = http.StatusCreated
 				returnBody = []byte(`{"warnings":[
 				  {"type": "warning-1-type", "message": "fake-warning1"},
@@ -245,6 +251,20 @@ var _ = Describe("ATC Handler Configs", func() {
 				It("returns an error", func() {
 					_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
 					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("client: unable to JSON parse the response body"))
+				})
+			})
+
+			Context("when response does not contain application/json header", func() {
+				BeforeEach(func() {
+					contentType = "text/plain"
+					returnBody = []byte(`server error`)
+				})
+
+				It("returns an error", func() {
+					_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("server error"))
 				})
 			})
 
@@ -286,6 +306,7 @@ var _ = Describe("ATC Handler Configs", func() {
 
 		Context("when updating a config", func() {
 			BeforeEach(func() {
+				contentType = "application/json"
 				returnHeader = http.StatusOK
 				returnBody = []byte(`{"warnings":[
 				  {"type": "warning-1-type", "message": "fake-warning1"},
@@ -318,6 +339,7 @@ var _ = Describe("ATC Handler Configs", func() {
 				It("returns an error", func() {
 					_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
 					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("client: unable to JSON parse the response body"))
 				})
 			})
 
@@ -359,6 +381,7 @@ var _ = Describe("ATC Handler Configs", func() {
 
 		Context("when setting config returns bad request", func() {
 			BeforeEach(func() {
+				contentType = "application/json"
 				returnHeader = http.StatusBadRequest
 				returnBody = []byte(`{"errors":["fake-error1","fake-error2"]}`)
 			})
@@ -378,6 +401,20 @@ var _ = Describe("ATC Handler Configs", func() {
 				It("returns an error", func() {
 					_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
 					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("client: unable to JSON parse the response body"))
+				})
+			})
+
+			Context("when response does not contain application/json header", func() {
+				BeforeEach(func() {
+					contentType = "text/plain"
+					returnBody = []byte(`server error`)
+				})
+
+				It("returns an error", func() {
+					_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("server error"))
 				})
 			})
 		})