From 46898ca52c9cda217eb5680fb65cd8f7822c524d Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Fri, 27 Aug 2021 18:12:35 -0700 Subject: [PATCH] Return clearer errors when __typename is missing (#68) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: In practice, at Khan at least, this is easy to mess up when writing mocks, because you write the mock by looking at the query, and the query doesn't say it's asking for `__typename` (because genqlient automatically adds that). A sufficiently-smart mocking library might be able to fix that, or detect it at least, but in any case, we can give a clearer error. I also removed an unrelated TODO that was done. Issue: https://khanacademy.slack.com/archives/C01120CNCS0/p1630019788014000 ## Test plan: make check Author: benjaminjkraft Reviewers: dnerdy, aberkan, csilvers, MiguelCastillo Required Reviewers: Approved by: dnerdy Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint Pull request URL: https://github.com/Khan/genqlient/pull/68 --- ...Field.graphql-InterfaceListField.graphql.go | 12 ++++++++++-- ...InterfaceListOfListsOfListsField.graphql.go | 12 ++++++++++-- ...Nesting.graphql-InterfaceNesting.graphql.go | 12 ++++++++++-- ...nts.graphql-InterfaceNoFragments.graphql.go | 18 +++++++++++++++--- ...agments.graphql-UnionNoFragments.graphql.go | 6 +++++- generate/unmarshal.go.tmpl | 3 ++- generate/unmarshal_helper.go.tmpl | 7 ++++++- internal/integration/generated.go | 18 +++++++++++++++--- 8 files changed, 73 insertions(+), 15 deletions(-) diff --git a/generate/testdata/snapshots/TestGenerate-InterfaceListField.graphql-InterfaceListField.graphql.go b/generate/testdata/snapshots/TestGenerate-InterfaceListField.graphql-InterfaceListField.graphql.go index df0cea5..c5619bd 100644 --- a/generate/testdata/snapshots/TestGenerate-InterfaceListField.graphql-InterfaceListField.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-InterfaceListField.graphql-InterfaceListField.graphql.go @@ -50,7 +50,8 @@ func (v *InterfaceListFieldRootTopic) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceListFieldRootTopicChildrenContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceListFieldRootTopic.Children: %w", err) } } } @@ -147,6 +148,9 @@ func __unmarshalInterfaceListFieldRootTopicChildrenContent(v *InterfaceListField case "Topic": *v = new(InterfaceListFieldRootTopicChildrenTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceListFieldRootTopicChildrenContent: "%v"`, tn.TypeName) @@ -203,7 +207,8 @@ func (v *InterfaceListFieldWithPointerTopic) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceListFieldWithPointerTopicChildrenContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceListFieldWithPointerTopic.Children: %w", err) } } } @@ -300,6 +305,9 @@ func __unmarshalInterfaceListFieldWithPointerTopicChildrenContent(v *InterfaceLi case "Topic": *v = new(InterfaceListFieldWithPointerTopicChildrenTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceListFieldWithPointerTopicChildrenContent: "%v"`, tn.TypeName) diff --git a/generate/testdata/snapshots/TestGenerate-InterfaceListOfListsOfListsField.graphql-InterfaceListOfListsOfListsField.graphql.go b/generate/testdata/snapshots/TestGenerate-InterfaceListOfListsOfListsField.graphql-InterfaceListOfListsOfListsField.graphql.go index f91a299..ffcc237 100644 --- a/generate/testdata/snapshots/TestGenerate-InterfaceListOfListsOfListsField.graphql-InterfaceListOfListsOfListsField.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-InterfaceListOfListsOfListsField.graphql-InterfaceListOfListsOfListsField.graphql.go @@ -110,6 +110,9 @@ func __unmarshalInterfaceListOfListOfListsFieldListOfListsOfListsOfContent(v *In case "Topic": *v = new(InterfaceListOfListOfListsFieldListOfListsOfListsOfContentTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceListOfListOfListsFieldListOfListsOfListsOfContent: "%v"`, tn.TypeName) @@ -183,7 +186,8 @@ func (v *InterfaceListOfListOfListsFieldResponse) UnmarshalJSON(b []byte) error err = __unmarshalInterfaceListOfListOfListsFieldListOfListsOfListsOfContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceListOfListOfListsFieldResponse.ListOfListsOfListsOfContent: %w", err) } } } @@ -211,7 +215,8 @@ func (v *InterfaceListOfListOfListsFieldResponse) UnmarshalJSON(b []byte) error err = __unmarshalInterfaceListOfListOfListsFieldWithPointerContent( *target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceListOfListOfListsFieldResponse.WithPointer: %w", err) } } } @@ -310,6 +315,9 @@ func __unmarshalInterfaceListOfListOfListsFieldWithPointerContent(v *InterfaceLi case "Topic": *v = new(InterfaceListOfListOfListsFieldWithPointerTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceListOfListOfListsFieldWithPointerContent: "%v"`, tn.TypeName) diff --git a/generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.go b/generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.go index 336bf1d..1cb1d33 100644 --- a/generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.go @@ -48,7 +48,8 @@ func (v *InterfaceNestingRootTopic) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceNestingRootTopicChildrenContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceNestingRootTopic.Children: %w", err) } } } @@ -151,6 +152,9 @@ func __unmarshalInterfaceNestingRootTopicChildrenContent(v *InterfaceNestingRoot case "Topic": *v = new(InterfaceNestingRootTopicChildrenTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceNestingRootTopicChildrenContent: "%v"`, tn.TypeName) @@ -190,7 +194,8 @@ func (v *InterfaceNestingRootTopicChildrenParentTopic) UnmarshalJSON(b []byte) e err = __unmarshalInterfaceNestingRootTopicChildrenParentTopicChildrenContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceNestingRootTopicChildrenParentTopic.Children: %w", err) } } } @@ -283,6 +288,9 @@ func __unmarshalInterfaceNestingRootTopicChildrenParentTopicChildrenContent(v *I case "Topic": *v = new(InterfaceNestingRootTopicChildrenParentTopicChildrenTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceNestingRootTopicChildrenParentTopicChildrenContent: "%v"`, tn.TypeName) diff --git a/generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.go b/generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.go index a581c53..10b0aa1 100644 --- a/generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.go @@ -100,6 +100,9 @@ func __unmarshalInterfaceNoFragmentsQueryRandomItemContent(v *InterfaceNoFragmen case "Topic": *v = new(InterfaceNoFragmentsQueryRandomItemTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceNoFragmentsQueryRandomItemContent: "%v"`, tn.TypeName) @@ -218,6 +221,9 @@ func __unmarshalInterfaceNoFragmentsQueryRandomItemWithTypeNameContent(v *Interf case "Topic": *v = new(InterfaceNoFragmentsQueryRandomItemWithTypeNameTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceNoFragmentsQueryRandomItemWithTypeNameContent: "%v"`, tn.TypeName) @@ -271,7 +277,8 @@ func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceNoFragmentsQueryRandomItemContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceNoFragmentsQueryResponse.RandomItem: %w", err) } } { @@ -280,7 +287,8 @@ func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceNoFragmentsQueryRandomItemWithTypeNameContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceNoFragmentsQueryResponse.RandomItemWithTypeName: %w", err) } } { @@ -290,7 +298,8 @@ func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error { err = __unmarshalInterfaceNoFragmentsQueryWithPointerContent( *target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal InterfaceNoFragmentsQueryResponse.WithPointer: %w", err) } } return nil @@ -393,6 +402,9 @@ func __unmarshalInterfaceNoFragmentsQueryWithPointerContent(v *InterfaceNoFragme case "Topic": *v = new(InterfaceNoFragmentsQueryWithPointerTopic) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Content.__typename") default: return fmt.Errorf( `Unexpected concrete type for InterfaceNoFragmentsQueryWithPointerContent: "%v"`, tn.TypeName) diff --git a/generate/testdata/snapshots/TestGenerate-UnionNoFragments.graphql-UnionNoFragments.graphql.go b/generate/testdata/snapshots/TestGenerate-UnionNoFragments.graphql-UnionNoFragments.graphql.go index 43fe75b..da7013d 100644 --- a/generate/testdata/snapshots/TestGenerate-UnionNoFragments.graphql-UnionNoFragments.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-UnionNoFragments.graphql-UnionNoFragments.graphql.go @@ -61,6 +61,9 @@ func __unmarshalUnionNoFragmentsQueryRandomLeafLeafContent(v *UnionNoFragmentsQu case "Video": *v = new(UnionNoFragmentsQueryRandomLeafVideo) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing LeafContent.__typename") default: return fmt.Errorf( `Unexpected concrete type for UnionNoFragmentsQueryRandomLeafLeafContent: "%v"`, tn.TypeName) @@ -98,7 +101,8 @@ func (v *UnionNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error { err = __unmarshalUnionNoFragmentsQueryRandomLeafLeafContent( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal UnionNoFragmentsQueryResponse.RandomLeaf: %w", err) } } return nil diff --git a/generate/unmarshal.go.tmpl b/generate/unmarshal.go.tmpl index 980d3b6..d91a7f6 100644 --- a/generate/unmarshal.go.tmpl +++ b/generate/unmarshal.go.tmpl @@ -85,7 +85,8 @@ func (v *{{.GoName}}) UnmarshalJSON(b []byte) error { err = __unmarshal{{$field.GoType.Unwrap.Reference}}( {{if $field.GoType.IsPointer}}*{{end}}target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal {{$.GoName}}.{{$field.GoName}}: %w", err) } {{range $i := intRange $field.GoType.SliceDepth -}} } diff --git a/generate/unmarshal_helper.go.tmpl b/generate/unmarshal_helper.go.tmpl index 8916375..a2b721f 100644 --- a/generate/unmarshal_helper.go.tmpl +++ b/generate/unmarshal_helper.go.tmpl @@ -17,10 +17,15 @@ func __unmarshal{{.GoName}}(v *{{.GoName}}, m {{ref "encoding/json.RawMessage"}} switch tn.TypeName { {{range .Implementations -}} case "{{.GraphQLName}}": - {{/* TODO: handle repeated fields! */ -}} *v = new({{.GoName}}) return {{ref "encoding/json.Unmarshal"}}(m, *v) {{end -}} + case "": + {{/* Likely if we're making a request to a mock server and the author + of the mock didn't know to add __typename, so give a special + error. */ -}} + return {{ref "fmt.Errorf"}}( + "Response was missing {{.GraphQLName}}.__typename") default: return {{ref "fmt.Errorf"}}( `Unexpected concrete type for {{.GoName}}: "%v"`, tn.TypeName) diff --git a/internal/integration/generated.go b/internal/integration/generated.go index 8cf8333..c0fcb08 100644 --- a/internal/integration/generated.go +++ b/internal/integration/generated.go @@ -327,6 +327,9 @@ func __unmarshalqueryWithInterfaceListFieldBeingsBeing(v *queryWithInterfaceList case "Animal": *v = new(queryWithInterfaceListFieldBeingsAnimal) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Being.__typename") default: return fmt.Errorf( `Unexpected concrete type for queryWithInterfaceListFieldBeingsBeing: "%v"`, tn.TypeName) @@ -371,7 +374,8 @@ func (v *queryWithInterfaceListFieldResponse) UnmarshalJSON(b []byte) error { err = __unmarshalqueryWithInterfaceListFieldBeingsBeing( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal queryWithInterfaceListFieldResponse.Beings: %w", err) } } } @@ -448,6 +452,9 @@ func __unmarshalqueryWithInterfaceListPointerFieldBeingsBeing(v *queryWithInterf case "Animal": *v = new(queryWithInterfaceListPointerFieldBeingsAnimal) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Being.__typename") default: return fmt.Errorf( `Unexpected concrete type for queryWithInterfaceListPointerFieldBeingsBeing: "%v"`, tn.TypeName) @@ -493,7 +500,8 @@ func (v *queryWithInterfaceListPointerFieldResponse) UnmarshalJSON(b []byte) err err = __unmarshalqueryWithInterfaceListPointerFieldBeingsBeing( *target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal queryWithInterfaceListPointerFieldResponse.Beings: %w", err) } } } @@ -563,6 +571,9 @@ func __unmarshalqueryWithInterfaceNoFragmentsBeing(v *queryWithInterfaceNoFragme case "Animal": *v = new(queryWithInterfaceNoFragmentsBeingAnimal) return json.Unmarshal(m, *v) + case "": + return fmt.Errorf( + "Response was missing Being.__typename") default: return fmt.Errorf( `Unexpected concrete type for queryWithInterfaceNoFragmentsBeing: "%v"`, tn.TypeName) @@ -616,7 +627,8 @@ func (v *queryWithInterfaceNoFragmentsResponse) UnmarshalJSON(b []byte) error { err = __unmarshalqueryWithInterfaceNoFragmentsBeing( target, raw) if err != nil { - return err + return fmt.Errorf( + "Unable to unmarshal queryWithInterfaceNoFragmentsResponse.Being: %w", err) } } return nil