https://kotlinlang.org logo
#apollo-kotlin
Title
# apollo-kotlin
s

Stylianos Gakis

06/20/2022, 1:51 PM
If I am hitting the error “Expected END_OBJECT but was ${peek()} at path ${getPathAsString()}” in code where I am not doing anything fancy (Just executing a query) what would you suggest I start looking into first? Is there anything obvious I could be messing up 🤔 Having a hard time figuring it out.
m

mbonnin

06/20/2022, 1:52 PM
Could it be that your server sends extra fields?
What does
peek()
say?
s

Stylianos Gakis

06/20/2022, 1:56 PM
Got a failing test here to be able to debug this, with the response as it comes from the backend in the string right below. Exact crash is: “Expected END_OBJECT but was NAME at path data.embarkStory.passages.5.response.__typename” Got the response from here with variables:
Copy code
{
  "name": "Web Onboarding SE - Quote Cart Needer?quoteCartId=65db82c7-0475-4793-84b8-57a1562cde5b",
  "locale": "en_SE"
}
And Query:
Copy code
query EmbarkStory($name: String!, $locale: String!) { embarkStory(name: $name, locale: $locale) { startPassage computedStoreValues { key value } passages { name id externalRedirect { data { location } } offerRedirect { data { keys } } messages { __typename ...MessageFragment } response { __typename ...MessageFragment ...ResponseExpressionFragment ... on EmbarkGroupedResponse { title { __typename ...ResponseExpressionFragment } items { __typename ...MessageFragment } each { key content { __typename ...MessageFragment } } } } redirects { __typename ... on EmbarkRedirectUnaryExpression { unaryType: type to passedExpressionKey passedExpressionValue } ... on EmbarkRedirectBinaryExpression { binaryType: type to key value passedExpressionKey passedExpressionValue } ... on EmbarkRedirectMultipleExpressions { multipleExpressionType: type to passedExpressionKey passedExpressionValue subExpressions { __typename ...ExpressionFragment } } } action { __typename ... on EmbarkSelectAction { selectData: data { options { link { __typename ...EmbarkLinkFragment } keys values badge api { __typename ...ApiFragment } } } } ... on EmbarkTextAction { textData: data { key placeholder mask subtitle link { __typename ...EmbarkLinkFragment } api { __typename ...ApiFragment } } } ... on EmbarkTextActionSet { textSetData: data { link { __typename ...EmbarkLinkFragment } textActions { data { key mask placeholder title } } api { __typename ...ApiFragment } } } ... on EmbarkPreviousInsuranceProviderAction { previousInsurerData: data { next { __typename ...EmbarkLinkFragment } skip { __typename ...EmbarkLinkFragment } storeKey insuranceProviders { id name logo { variants { __typename ...IconVariantsFragment } } } } } ... on EmbarkExternalInsuranceProviderAction { externalInsurerData: data { next { __typename ...EmbarkLinkFragment } skip { __typename ...EmbarkLinkFragment } storeKey } } ... on EmbarkNumberAction { numberActionData: data { __typename ...EmbarkNumberActionFragment } } ... on EmbarkNumberActionSet { numberActionSetData: data { numberActions { data { key placeholder unit label maxValue minValue title } } link { __typename ...EmbarkLinkFragment } } } ... on EmbarkDatePickerAction { storeKey label next { __typename ...EmbarkLinkFragment } } ... on EmbarkMultiAction { multiActionData: data { key maxAmount addLabel link { __typename ...EmbarkLinkFragment } components { __typename ... on EmbarkMultiActionNumberAction { numberActionData: data { key placeholder label unit } } ... on EmbarkDropdownAction { dropDownActionData: data { label key options { value text } } } ... on EmbarkSwitchAction { switchActionData: data { label key defaultValue } } } } } ... on EmbarkAudioRecorderAction { audioRecorderActionData: data { storeKey label next { __typename ...EmbarkLinkFragment } } } ... on EmbarkAddressAutocompleteAction { component addressAutocompleteActionData: data { placeholder key link { __typename ...EmbarkLinkFragment } api { __typename ...ApiFragment } } } } api { __typename ...ApiFragment } tooltips { title description } allLinks { __typename ...EmbarkLinkFragment } tracks { eventName eventKeys includeAllKeys customData } quoteCartOfferRedirects { data { id expression { __typename ...ExpressionFragment } selectedInsuranceTypes } } variantedOfferRedirects { data { selectedKeys allKeys expression { __typename ...ExpressionFragment } } } } } }  fragment BasicExpressionFragment on EmbarkExpression { __typename ... on EmbarkExpressionUnary { unaryType: type text } ... on EmbarkExpressionBinary { binaryType: type key value text } }  fragment ExpressionFragment on EmbarkExpression { __typename ...BasicExpressionFragment ... on EmbarkExpressionMultiple { multipleType: type text subExpressions { __typename ...BasicExpressionFragment ... on EmbarkExpressionMultiple { multipleType: type text subExpressions { __typename ...BasicExpressionFragment ... on EmbarkExpressionMultiple { multipleType: type text subExpressions { __typename ...BasicExpressionFragment } } } } } } }  fragment MessageFragment on EmbarkMessage { expressions { __typename ...ExpressionFragment } text }  fragment ResponseExpressionFragment on EmbarkResponseExpression { text expressions { __typename ...ExpressionFragment } }  fragment EmbarkLinkFragment on EmbarkLink { name label hidden }  fragment GraphQLResultsFragment on EmbarkAPIGraphQLResult { key as }  fragment GraphQLErrorsFragment on EmbarkAPIGraphQLError { contains next { __typename ...EmbarkLinkFragment } }  fragment GraphQLVariablesFragment on EmbarkAPIGraphQLVariable { __typename ... on EmbarkAPIGraphQLSingleVariable { key from as } ... on EmbarkAPIGraphQLGeneratedVariable { key storeAs type } ... on EmbarkAPIGraphQLMultiActionVariable { key from variables { __typename ... on EmbarkAPIGraphQLSingleVariable { key from as } ... on EmbarkAPIGraphQLGeneratedVariable { key storeAs type } } } ... on EmbarkAPIGraphQLConstantVariable { key value as } }  fragment ApiFragment on EmbarkApi { __typename ... on EmbarkApiGraphQLQuery { queryData: data { query results { __typename ...GraphQLResultsFragment } errors { __typename ...GraphQLErrorsFragment } variables { __typename ...GraphQLVariablesFragment } next { __typename ...EmbarkLinkFragment } } } ... on EmbarkApiGraphQLMutation { mutationData: data { mutation results { __typename ...GraphQLResultsFragment } variables { __typename ...GraphQLVariablesFragment } errors { __typename ...GraphQLErrorsFragment } next { __typename ...EmbarkLinkFragment } } } }  fragment IconVariantsFragment on IconVariants { dark { svgUrl } light { svgUrl } }  fragment EmbarkNumberActionFragment on EmbarkNumberActionData { key placeholder unit label maxValue minValue link { __typename ...EmbarkLinkFragment } }
Sorry just wrote the above for myself too, to easily reproduce if we don’t figure it out 😄
👍 1
NAME is what I get back doing .peek() on the actualReader right before the crash
m

mbonnin

06/20/2022, 2:15 PM
I get this error trying to run the test:
Copy code
java.io.FileNotFoundException: /Users/mbonnin/git/hedvig-android/apollo/src/main/graphql/com/hedvig/android/owldroid/schema.graphqls (No such file or directory)
I'll try with the one from playground
s

Stylianos Gakis

06/20/2022, 2:17 PM
And no I don’t see any extra fields that the backend is returning. The one which is failed to be parsed at position 5 seems to be this:
Copy code
"response": {
  "__typename": "EmbarkGroupedResponse",
  "title": {
    "__typename": "EmbarkResponseExpression",
    "text": "Extra buildings",
    "expressions": []
  },
  "items": [],
  "each": {
    "key": "extraBuildings",
    "content": {
      "__typename": "EmbarkMessage",
      "expressions": [],
      "text": "{type.Label}, {area} square meters"
    }
  }
}
Which seems to be the correct ones according to the schema, since the response is EmbarkGroupedResponse
Copy code
... on EmbarkGroupedResponse {
  title {
    ...ResponseExpressionFragment
  }
  items {
    ...MessageFragment
  }
  each {
    key
    content {
      ...MessageFragment
    }
  }
}
Did you try running first ./gradlew apollo:downloadGiraffeApolloSchemaFromIntrospection Our schema isn’t checked out in the repo
👍 1
m

mbonnin

06/20/2022, 2:18 PM
I didn't, I just ran the test
👌 1
Damn, looks like I need the lokalize strings to build the app. Let me see if I can move the test to a non-android module
s

Stylianos Gakis

06/20/2022, 2:26 PM
Ah right there’s that too. Benoit last time went around this by providing some fake values to make the build happy, since that step is not part of the build really, but a gradle task to download the strings, you don’t need that there afaik
m

mbonnin

06/20/2022, 2:27 PM
maybe I can regex my way out of this
s

Stylianos Gakis

06/20/2022, 2:27 PM
I am sorry there’s this friction, I need to try and go through this process myself some time to make this setup as effortless as I can for people trying to check the repo out
What exactly is the issue maybe I can help you with by sending you some stuff over on a DM
m

mbonnin

06/20/2022, 2:29 PM
Copy code
Execution failed for task ':app:processDebugResources'.
> A failure occurred while executing com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$TaskAction
   > Android resource linking failed
     com.hedvig.app-mergeDebugResources-111:/values/values.xml:24: error: resource string/SETTINGS_LANGUAGE_SWEDISH (aka <http://com.hedvig.dev.app:string/SETTINGS_LANGUAGE_SWEDISH|com.hedvig.dev.app:string/SETTINGS_LANGUAGE_SWEDISH>) not found.
b

bod

06/20/2022, 2:30 PM
ooh yeah last time you simply sent me the strings.xml by dm 🙂
👍 1
s

Stylianos Gakis

06/20/2022, 2:31 PM
Yeap just sent them in a DM to Martin as well
🙏 1
m

mbonnin

06/20/2022, 2:51 PM
So there are no extra fields sent by the server but since the json reader was rewinded to parse the fragments, it still sees some 😕
s

Stylianos Gakis

06/20/2022, 2:53 PM
Interesting! Is it due to this combination of inline fragments and real fragments at the same time? Seems like a very complicated combination to get everything right. I am glad I could give you a repro of this bug though, at least we know what’s happening now.
m

mbonnin

06/20/2022, 2:53 PM
Yea, I think this only happens if there are both inline and named fragment
s

Stylianos Gakis

06/20/2022, 2:54 PM
Okay let me try replacing the inline fragment with a real one and see if it crashes
🙏 1
m

mbonnin

06/20/2022, 3:00 PM
🥳
s

Stylianos Gakis

06/20/2022, 3:00 PM
Do you want me to make a proper error in the apollo-kotlin repository to make it easier for you?
Also thanks so much for taking the time to help me once again! I appreciate it a ton!
m

mbonnin

06/20/2022, 3:01 PM
Thank you for the precious feedback!
I can make a small reproducer in the repo
I think it shouldn't be too hard.
Maybe the good fix is to actually ignore the error....
s

Stylianos Gakis

06/20/2022, 3:04 PM
Okay so no need for a new GitHub issue I guess. I’d like to be able to follow it to see updates on it. I am now a bit paranoid that we may be doing inline and normal fragments in more places in the app, and it might be a bit easier for me to just wait for a fix 😄 We’ve not merged the 3.x changes in our app yet, but I am actively trying to make that happen atm.
yeah it might be, why was the error there in the first place? Was it preventing some wrong behavior or more that it was a state that you thought never should have happened but it does in fact happen in this very specific scenario?
m

mbonnin

06/20/2022, 3:04 PM
Oh yes, please open an issue
👍 1
Issues are always good!
We might make a hotfix release for that actually, not sure, still trying to wrap my head around what this implies
Was it preventing some wrong behavior or more that it was a state that you thought never should have happened but it does in fact happen in this very specific scenario?
It is required to make https://github.com/apollographql/apollo-kotlin/issues/3344 work
(although we have not added the strict mode yet... but it feels like an interesting thing to add)
Making that work with fragments and operationBased models sounds.... interesting...
s

Stylianos Gakis

06/20/2022, 3:10 PM
Yeap I see. Haha yeah it all sounds very interesting, and hard 🤣
m

mbonnin

06/20/2022, 3:10 PM
With
responseBased
models, it's not an issue because we read all the fields all the time. But with
operationBased
models and fragments only having "local" knowledge of the selection set, it feels a lot more challenging...
Maybe the tradeoff is "no strict mode for
operationBased
models"
Or we need to add more state to
MapJsonReader
for the strict mode 🤔
s

Stylianos Gakis

06/20/2022, 3:21 PM
Added this issue and linked to this discussion too! I figured since I posted a lot of the repro steps etc in here it’s enough to add a rough description and then link here for the rest 😄
💙 1
m

mbonnin

06/20/2022, 3:22 PM
That's perfect, thanks!
s

Stylianos Gakis

06/20/2022, 3:26 PM
As far as your messages above, I am afraid I can’t quite follow all your thoughts, I am not that well versed in the library internals unfortunately to confidently give ideas regarding this. If anything, the “no strict mode for operationBased” sounds like a very reasonable workaround at least as a first step.
m

mbonnin

06/20/2022, 3:27 PM
Sure thing! Sorry for my ramblings!
I think we'll do "no strict mode for operationBased" at first and then add an "opt-in strict mode for operationBased" if we find out it's required
s

Stylianos Gakis

06/20/2022, 3:44 PM
Thinking out loud is very valuable, I wouldn't call it a rambling. I actually appreciate it when you do that, sometimes I learn stuff that way. And yeah that idea sounds very reasonable to me!
🙂 1
m

mbonnin

06/21/2022, 7:32 AM
I think it hasn't been a bigger issue because it only affects
compat
models but we should certainly make a release about this.
❤️ 1
s

Stylianos Gakis

06/21/2022, 7:57 AM
Right, there’s that which I have to take care of sometime as well I suppose. I dropped this because I was looking into responseBased back in the day but hit this problem and simply put it to the side ever since. I am thinking though maybe I should go into operationBased from compat at some point though. I didn’t see any mention of how one would do this move in the migration to v3 docs. I can see you say compat is “similar” to operationBased but is there a specification somewhere that can explain the differences to me if there are any?
m

mbonnin

06/21/2022, 7:58 AM
operationBased
drops the
.fragments
intermediate field and renames
asFoo
to
onFoo
. That's mostly it
(and doesn't collect inline fragments parent fields, hence the renaming)
s

Stylianos Gakis

06/21/2022, 8:00 AM
Aha I see! I’ll probably do this eventually then. Will also be another of these cases where I have to manually go and change a lot of code due to how we use these models 😅
I am sorry I don’t think I understand what “doesn’t collect inline fragments parent fields” means 🧐
m

mbonnin

06/21/2022, 8:04 AM
Copy code
{
   name
   ... on Droid {
      primaryFunction
   }
}
compat does:
Copy code
Hero(val name: String, val asDroid: AsDroid?)
AsDroid(val name: String, val primaryFunction: String)
operationBased does:
Copy code
Hero(val name: String, val onDroid: OnDroid?)
OnDroid(val primaryFunction: String)
s

Stylianos Gakis

06/21/2022, 8:07 AM
Aha gets rid of this duplication of data, nice this makes sense, thank you for explaining!
m

mbonnin

06/21/2022, 8:08 AM
We should certainly put this somewhere but this is hard TBH
s

Stylianos Gakis

06/21/2022, 8:09 AM
Somewhere in the docs you mean? A section to give a hint to the migrators how to get off of the compat model generation? Or?
m

mbonnin

06/21/2022, 8:09 AM
Yes, document it somewhere
For migrations, this would ideally be a intelliJ refactoring. But we're nowhere near that unfortunately...
s

Stylianos Gakis

06/21/2022, 8:14 AM
I just saw the design doc does explain these differences. The super simple way would be in the “Migrating to Apollo Kotlin 3.0” doc to have a section about why
codegenModels.set(MODELS_COMPAT)
exists and link to the design doc. And yeah an IntelliJ refactoring would be optimal, but it sounds quite complicated to do it.
m

mbonnin

06/21/2022, 8:18 AM
Great idea. I'll do this 👍
👏 1
s

Stylianos Gakis

06/21/2022, 9:18 AM
Yep that’s perfect! Great job 🥳
m

mbonnin

06/21/2022, 9:21 AM
Thanks for rising this 🙂
🙌 1
24 Views