Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SimpleResponseParser fails when directives reference absent booleans with default values #2703

Closed
DSteve595 opened this issue Oct 27, 2020 · 9 comments · Fixed by #2704
Closed

SimpleResponseParser fails when directives reference absent booleans with default values #2703

DSteve595 opened this issue Oct 27, 2020 · 9 comments · Fixed by #2704
Assignees
@martinbonnin
Labels
🐛 Type: Bug ⚙️ Codegen

Comments

@DSteve595
Copy link
Contributor

Version
2.4.1

Description
Given a query that has a boolean arg with a default value, e.g.

query MyQuery($myBool: Boolean = true) {
    getCityByName(name: "London") {
        id @include(if: $myBool)
    }
}

Parsing this query via ApolloClient works as expected. However, using Operation::parse fails with this exception:

kotlin.TypeCastException: null cannot be cast to non-null type kotlin.Boolean
	at com.apollographql.apollo.api.internal.SimpleResponseReader.shouldSkip(SimpleResponseReader.kt:133)
	at com.apollographql.apollo.api.internal.SimpleResponseReader.readCustomType(SimpleResponseReader.kt:99)
	at MyQuery$GetCityByName$Companion.invoke(MyQuery.kt:123)
	at MyQuery$Data$Companion$invoke$1$getCityByName$1.invoke(MyQuery.kt:153)
	at MyQuery$Data$Companion$invoke$1$getCityByName$1.invoke(MyQuery.kt:145)
	at com.apollographql.apollo.api.internal.ResponseReader$readObject$1.read(ResponseReader.kt:26)
	at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:80)
	at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readObject(ResponseReader.kt:24)
	at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:11)
	at MyQuery$Data$Companion.invoke(MyQuery.kt:152)
	at MyQuery$responseFieldMapper$$inlined$invoke$1.map(ResponseFieldMapper.kt:22)
	at com.apollographql.apollo.api.internal.SimpleOperationResponseParser.parse(SimpleOperationResponseParser.kt:39)
	at com.apollographql.apollo.api.internal.SimpleOperationResponseParser.parse(SimpleOperationResponseParser.kt:26)
	at MyQuery.parse(MyQuery.kt:66)
	at MyQuery.parse(MyQuery.kt:73)

Sample project

Run ./gradlew run to trigger the issue.
@martinbonnin martinbonnin self-assigned this Oct 27, 2020
@martinbonnin
Copy link
Contributor

Thanks for filing this. It looks related to #2686

martinbonnin added a commit that referenced this issue Oct 27, 2020
@martinbonnin
Add support for default variable values.
a9ab41a 
This only works for scalar types and their list/optional variants, not
input object types

closes #2703 and part of #2686
@martinbonnin martinbonnin mentioned this issue Oct 27, 2020
Merged
@martinbonnin
Copy link
Contributor

See pull request in #2704 for a fix.

Side note: for this particular exemple, the spec recommends making the Boolean as non-nullable to avoid ambiguity.
It would be nice to output a warning during validation in these cases.

# myBool should be non-optional here
query MyQuery($myBool: Boolean! = true) {
    getCityByName(name: "London") {
        id @include(if: $myBool)
    }
}

@DSteve595
Copy link
Contributor Author

Making the boolean non-nullable is what made sense to me, so it's actually what I tried first. But (at least on GraphiQL), I get this:
image

@martinbonnin
Copy link
Contributor

Interesting. Playground does the same indeed. Looking at this other issue it looks like this is more of a tooling issue. Does the query run if you hit play despite the red warning?

@DSteve595
Copy link
Contributor Author

Nope, same error:

{
  "message": "Query validation error: Invalid query: ['Variable \"$includeSubredditInPosts\" of type \"Boolean!\" is required and will not use the default value. Perhaps you meant to use type \"Boolean\".']"
}

@martinbonnin
Copy link
Contributor

Interesting. Optional it is then! And certainly not worth adding a warning in the compiler if fixing the warning breaks the query 😬

martinbonnin added a commit that referenced this issue Oct 28, 2020
@martinbonnin
Add support for default variable values. (#2704)
d9039bf 
This only works for scalar types and their list/optional variants, not
input object types

closes #2703 and part of #2686
@DSteve595
Copy link
Contributor Author

Hmm.. I can confirm that 2.4.2 fixes this in my sample project, but I'm still seeing the error in a bigger project. Here's the full stack trace. If more info is required (like the full query), would prefer to message privately.

kotlin.TypeCastException: null cannot be cast to non-null type kotlin.Boolean
    at com.apollographql.apollo.api.internal.SimpleResponseReader.shouldSkip(SimpleResponseReader.kt:133)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:72)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readObject(ResponseReader.kt:24)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:11)
    at com.reddit.fragment.PostContentFragment$AsSubredditPost$Companion.invoke(PostContentFragment.kt:2583)
    at com.reddit.fragment.PostContentFragment$Companion$invoke$1$asSubredditPost$1.invoke(PostContentFragment.kt:525)
    at com.reddit.fragment.PostContentFragment$Companion$invoke$1$asSubredditPost$1.invoke(PostContentFragment.kt:275)
    at com.apollographql.apollo.api.internal.ResponseReader$readFragment$1.read(ResponseReader.kt:36)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:125)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readFragment(ResponseReader.kt:34)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:11)
    at com.reddit.fragment.PostContentFragment$Companion.invoke(PostContentFragment.kt:524)
    at com.reddit.fragment.PostFragment$Fragments$Companion$invoke$1$postContentFragment$1.invoke(PostFragment.kt:91)
    at com.reddit.fragment.PostFragment$Fragments$Companion$invoke$1$postContentFragment$1.invoke(PostFragment.kt:84)
    at com.apollographql.apollo.api.internal.ResponseReader$readFragment$1.read(ResponseReader.kt:36)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:125)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readFragment(ResponseReader.kt:34)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:11)
    at com.reddit.fragment.PostFragment$Fragments$Companion.invoke(PostFragment.kt:90)
    at com.reddit.fragment.PostFragment$Companion.invoke(PostFragment.kt:65)
    at com.reddit.queries.PopularPostsQuery$Node$Fragments$Companion$invoke$1$postFragment$1.invoke(PopularPostsQuery.kt:277)
    at com.reddit.queries.PopularPostsQuery$Node$Fragments$Companion$invoke$1$postFragment$1.invoke(PopularPostsQuery.kt:270)
    at com.apollographql.apollo.api.internal.ResponseReader$readFragment$1.read(ResponseReader.kt:36)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:125)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readFragment(ResponseReader.kt:34)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readFragment(SimpleResponseReader.kt:11)
    at com.reddit.queries.PopularPostsQuery$Node$Fragments$Companion.invoke(PopularPostsQuery.kt:276)
    at com.reddit.queries.PopularPostsQuery$Node$Companion.invoke(PopularPostsQuery.kt:252)
    at com.reddit.queries.PopularPostsQuery$Edge$Companion$invoke$1$node$1.invoke(PopularPostsQuery.kt:311)
    at com.reddit.queries.PopularPostsQuery$Edge$Companion$invoke$1$node$1.invoke(PopularPostsQuery.kt:302)
    at com.apollographql.apollo.api.internal.ResponseReader$readObject$1.read(ResponseReader.kt:26)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:80)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readObject(ResponseReader.kt:24)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:11)
    at com.reddit.queries.PopularPostsQuery$Edge$Companion.invoke(PopularPostsQuery.kt:310)
    at com.reddit.queries.PopularPostsQuery$Posts$Companion$invoke$1$edges$1$1.invoke(PopularPostsQuery.kt:359)
    at com.reddit.queries.PopularPostsQuery$Posts$Companion$invoke$1$edges$1$1.invoke(PopularPostsQuery.kt:343)
    at com.apollographql.apollo.api.internal.ResponseReader$ListItemReader$readObject$1.read(ResponseReader.kt:80)
    at com.apollographql.apollo.api.internal.SimpleResponseReader$ListItemReader.readObject(SimpleResponseReader.kt:191)
    at com.apollographql.apollo.api.internal.ResponseReader$ListItemReader$DefaultImpls.readObject(ResponseReader.kt:78)
    at com.apollographql.apollo.api.internal.SimpleResponseReader$ListItemReader.readObject(SimpleResponseReader.kt:158)
    at com.reddit.queries.PopularPostsQuery$Posts$Companion$invoke$1$edges$1.invoke(PopularPostsQuery.kt:358)
    at com.reddit.queries.PopularPostsQuery$Posts$Companion$invoke$1$edges$1.invoke(PopularPostsQuery.kt:343)
    at com.apollographql.apollo.api.internal.ResponseReader$readList$1.read(ResponseReader.kt:46)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readList(SimpleResponseReader.kt:94)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readList(ResponseReader.kt:44)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readList(SimpleResponseReader.kt:11)
    at com.reddit.queries.PopularPostsQuery$Posts$Companion.invoke(PopularPostsQuery.kt:357)
    at com.reddit.queries.PopularPostsQuery$PostFeed$Companion$invoke$1$posts$1.invoke(PopularPostsQuery.kt:423)
    at com.reddit.queries.PopularPostsQuery$PostFeed$Companion$invoke$1$posts$1.invoke(PopularPostsQuery.kt:390)
    at com.apollographql.apollo.api.internal.ResponseReader$readObject$1.read(ResponseReader.kt:26)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:80)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readObject(ResponseReader.kt:24)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:11)
    at com.reddit.queries.PopularPostsQuery$PostFeed$Companion.invoke(PopularPostsQuery.kt:422)
    at com.reddit.queries.PopularPostsQuery$Data$Companion$invoke$1$postFeed$1.invoke(PopularPostsQuery.kt:459)
    at com.reddit.queries.PopularPostsQuery$Data$Companion$invoke$1$postFeed$1.invoke(PopularPostsQuery.kt:452)
    at com.apollographql.apollo.api.internal.ResponseReader$readObject$1.read(ResponseReader.kt:26)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:80)
    at com.apollographql.apollo.api.internal.ResponseReader$DefaultImpls.readObject(ResponseReader.kt:24)
    at com.apollographql.apollo.api.internal.SimpleResponseReader.readObject(SimpleResponseReader.kt:11)
    at com.reddit.queries.PopularPostsQuery$Data$Companion.invoke(PopularPostsQuery.kt:458)
    at com.reddit.queries.PopularPostsQuery$responseFieldMapper$$inlined$invoke$1.map(ResponseFieldMapper.kt:22)
    at com.apollographql.apollo.api.internal.SimpleOperationResponseParser.parse(SimpleOperationResponseParser.kt:39)
    at com.apollographql.apollo.api.internal.SimpleOperationResponseParser.parse(SimpleOperationResponseParser.kt:26)
    at com.reddit.queries.PopularPostsQuery.parse(PopularPostsQuery.kt:139)
    at com.reddit.queries.PopularPostsQuery.parse(PopularPostsQuery.kt:146)

@DSteve595
Copy link
Contributor Author

It seems to happen when passing Input.absent() as the arg. Since Input already has defined: Boolean, maybe the GraphQL-provided default arg should be used when defined == false?

@martinbonnin martinbonnin reopened this Nov 9, 2020
This was referenced Nov 10, 2020
Merged
Merged
Closed
@martinbonnin
Copy link
Contributor

Fixed with #2741

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martinbonnin martinbonnin

Labels
🐛 Type: Bug ⚙️ Codegen
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for default variable values. apollographql/apollo-kotlin
2 participants
@DSteve595 @martinbonnin