From 65fb0d3e82d71fd007ac85a7b2a40c167c5d9f81 Mon Sep 17 00:00:00 2001 From: George Blue Date: Fri, 19 Apr 2024 11:18:45 +0100 Subject: [PATCH] chore: handle some unhandled errors 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 926aae23..f29428d3 100644 --- a/domain/service_metadata.go +++ b/domain/service_metadata.go @@ -27,7 +27,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 fbb329d8..d4c8c34c 100644 --- a/domain/service_plan_metadata.go +++ b/domain/service_plan_metadata.go @@ -55,7 +55,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 {