From ef330bea876ea09ffd2a0450a85d46905db32fa8 Mon Sep 17 00:00:00 2001 From: George Blue Date: Fri, 19 Apr 2024 16:57:54 +0100 Subject: [PATCH] chore: handle some unhandled errors (#303) Some unhandled errors are now handled. Arguably this is not necessary as they are "impossible" errors that cannot occur and are untestable. But code can change over time, and if those errors ever become possible, then it will be better to fail fast rather than to ignore the error. --- domain/service_metadata.go | 4 +- domain/service_metadata_test.go | 148 ++++++++++++++++---------------- domain/service_plan_metadata.go | 4 +- 3 files changed, 79 insertions(+), 77 deletions(-) diff --git a/domain/service_metadata.go b/domain/service_metadata.go index 4d959e5f..d571c371 100644 --- a/domain/service_metadata.go +++ b/domain/service_metadata.go @@ -26,7 +26,9 @@ func (sm ServiceMetadata) MarshalJSON() ([]byte, error) { } var m map[string]interface{} - json.Unmarshal(b, &m) + if err := json.Unmarshal(b, &m); err != nil { + return nil, err + } delete(m, additionalMetadataName) for k, v := range sm.AdditionalMetadata { diff --git a/domain/service_metadata_test.go b/domain/service_metadata_test.go index 1ad8b2b9..3650c64c 100644 --- a/domain/service_metadata_test.go +++ b/domain/service_metadata_test.go @@ -10,20 +10,19 @@ import ( ) var _ = Describe("ServiceMetadata", func() { - Describe("ServiceMetadata", func() { - Describe("JSON encoding", func() { - It("uses the correct keys", func() { - shareable := true - metadata := domain.ServiceMetadata{ - DisplayName: "Cassandra", - LongDescription: "A long description of Cassandra", - DocumentationUrl: "doc", - SupportUrl: "support", - ImageUrl: "image", - ProviderDisplayName: "display", - Shareable: &shareable, - } - jsonString := `{ + Describe("JSON encoding", func() { + It("uses the correct keys", func() { + shareable := true + metadata := domain.ServiceMetadata{ + DisplayName: "Cassandra", + LongDescription: "A long description of Cassandra", + DocumentationUrl: "doc", + SupportUrl: "support", + ImageUrl: "image", + ProviderDisplayName: "display", + Shareable: &shareable, + } + jsonString := `{ "displayName":"Cassandra", "longDescription":"A long description of Cassandra", "documentationUrl":"doc", @@ -33,83 +32,83 @@ var _ = Describe("ServiceMetadata", func() { "shareable":true }` - Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) - }) + Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) + }) - It("encodes the AdditionalMetadata fields in the metadata fields", func() { - metadata := domain.ServiceMetadata{ - DisplayName: "name", - AdditionalMetadata: map[string]interface{}{ - "foo": "bar", - "baz": 1, - }, - } - jsonString := `{ + It("encodes the AdditionalMetadata fields in the metadata fields", func() { + metadata := domain.ServiceMetadata{ + DisplayName: "name", + AdditionalMetadata: map[string]interface{}{ + "foo": "bar", + "baz": 1, + }, + } + jsonString := `{ "displayName":"name", "foo": "bar", "baz": 1 }` - Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) + Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) - By("not mutating the AdditionalMetadata during custom JSON marshalling") - Expect(len(metadata.AdditionalMetadata)).To(Equal(2)) - }) + By("not mutating the AdditionalMetadata during custom JSON marshalling") + Expect(len(metadata.AdditionalMetadata)).To(Equal(2)) + }) - It("it can marshal same structure in parallel requests", func() { - metadata := domain.ServiceMetadata{ - DisplayName: "name", - AdditionalMetadata: map[string]interface{}{ - "foo": "bar", - "baz": 1, - }, - } - jsonString := `{ + It("it can marshal same structure in parallel requests", func() { + metadata := domain.ServiceMetadata{ + DisplayName: "name", + AdditionalMetadata: map[string]interface{}{ + "foo": "bar", + "baz": 1, + }, + } + jsonString := `{ "displayName":"name", "foo": "bar", "baz": 1 }` - var wg sync.WaitGroup - wg.Add(2) + var wg sync.WaitGroup + wg.Add(2) - for i := 0; i < 2; i++ { - go func() { - defer wg.Done() - defer GinkgoRecover() + for i := 0; i < 2; i++ { + go func() { + defer wg.Done() + defer GinkgoRecover() - Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) - }() - } - wg.Wait() - }) + Expect(json.Marshal(metadata)).To(MatchJSON(jsonString)) + }() + } + wg.Wait() + }) - It("returns an error when additional metadata is not marshallable", func() { - metadata := domain.ServiceMetadata{ - DisplayName: "name", - AdditionalMetadata: map[string]interface{}{ - "foo": make(chan int), - }, - } - _, err := json.Marshal(metadata) - Expect(err).To(MatchError(ContainSubstring("unmarshallable content in AdditionalMetadata"))) - }) + It("returns an error when additional metadata is not marshallable", func() { + metadata := domain.ServiceMetadata{ + DisplayName: "name", + AdditionalMetadata: map[string]interface{}{ + "foo": make(chan int), + }, + } + _, err := json.Marshal(metadata) + Expect(err).To(MatchError(ContainSubstring("unmarshallable content in AdditionalMetadata"))) }) + }) - Describe("JSON decoding", func() { - It("sets the AdditionalMetadata from unrecognized fields", func() { - metadata := domain.ServiceMetadata{} - jsonString := `{"foo":["test"],"bar":"Some display name"}` + Describe("JSON decoding", func() { + It("sets the AdditionalMetadata from unrecognized fields", func() { + metadata := domain.ServiceMetadata{} + jsonString := `{"foo":["test"],"bar":"Some display name"}` - err := json.Unmarshal([]byte(jsonString), &metadata) - Expect(err).NotTo(HaveOccurred()) - Expect(metadata.AdditionalMetadata["foo"]).To(Equal([]interface{}{"test"})) - Expect(metadata.AdditionalMetadata["bar"]).To(Equal("Some display name")) - }) + err := json.Unmarshal([]byte(jsonString), &metadata) + Expect(err).NotTo(HaveOccurred()) + Expect(metadata.AdditionalMetadata["foo"]).To(Equal([]interface{}{"test"})) + Expect(metadata.AdditionalMetadata["bar"]).To(Equal("Some display name")) + }) - It("does not include convention fields into additional metadata", func() { - metadata := domain.ServiceMetadata{} - jsonString := `{ + It("does not include convention fields into additional metadata", func() { + metadata := domain.ServiceMetadata{} + jsonString := `{ "displayName":"Cassandra", "longDescription":"A long description of Cassandra", "documentationUrl":"doc", @@ -118,10 +117,9 @@ var _ = Describe("ServiceMetadata", func() { "providerDisplayName":"display", "shareable":true }` - err := json.Unmarshal([]byte(jsonString), &metadata) - Expect(err).NotTo(HaveOccurred()) - Expect(metadata.AdditionalMetadata).To(BeNil()) - }) + err := json.Unmarshal([]byte(jsonString), &metadata) + Expect(err).NotTo(HaveOccurred()) + Expect(metadata.AdditionalMetadata).To(BeNil()) }) }) }) diff --git a/domain/service_plan_metadata.go b/domain/service_plan_metadata.go index 51b4b8a7..e8c5d6b1 100644 --- a/domain/service_plan_metadata.go +++ b/domain/service_plan_metadata.go @@ -54,7 +54,9 @@ func (spm ServicePlanMetadata) MarshalJSON() ([]byte, error) { } var m map[string]interface{} - json.Unmarshal(b, &m) + if err := json.Unmarshal(b, &m); err != nil { + return nil, err + } delete(m, additionalMetadataName) for k, v := range spm.AdditionalMetadata {