houdini: Union in response type causes invalid null cascade

Describe the bug

Two different operations, using inline fragments, interfere with each other and doing a store reset.

In my project I use Pothos to build schema.

And I especially use the pothos validation plugin + zod in my resolvers and pothos errors plugin which generate types at runtime for different responses. So in case of success it responds with one type and in case of error with another.

Inline fragments reference: https://graphql.org/learn/queries/#inline-fragments Union-types reference: https://graphql.org/learn/schema/#union-types

Severity

blocking all usage of Houdini

Steps to Reproduce the Bug

1- clone the repo

git clone https://github.com/524c/houdini-issue-twoOperations

then

cd houdini-issue-twoOperations
pnpm i
pnpm dev

check there

http://localhost:3000/twoOperations

Reproduction

The videos below are using the current version 0.17.13 and a simple Svelte project page.

Each button does a different query. The expected behavior is that the result would be persisted in the store, but one operation interferes with the other. Fisrt video without any changes in Houdini code:

https://user-images.githubusercontent.com/527641/204096718-beffa950-008d-42d0-bd72-a1ba878d1eec.mp4

This second video is after commenting the refreshSubscription function. That way I managed to keep the data of both operations.

Note: in this quick test I made direct changes to the volatile runtime files. $houdini/plugins/houdini-svelte/runtime/stores/query.js

  refreshSubscription(newVariables) {
    /*
    const cache = getCache();
    if (this.subscriptionSpec) {
      cache.unsubscribe(this.subscriptionSpec, this.lastVariables || {});
    }
    this.subscriptionSpec = {
      rootType: this.artifact.rootType,
      selection: this.artifact.selection,
      variables: () => newVariables,
      set: (newValue) => this.store.update((s) => ({ ...s, data: newValue }))
    };
    cache.subscribe(this.subscriptionSpec, newVariables);
    this.lastVariables = newVariables;*/
  }

Result after this “code change”:

https://user-images.githubusercontent.com/527641/204097169-08e1d188-492d-4abc-b68d-c7fa8b47c285.mp4

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 21 (12 by maintainers)

Commits related to this issue

:children_crossing: UPDATE: tentative repro #731 — committed to HoudiniGraphql/houdini by jycouet 2 years ago
Improve support for unions and interfaces (#747) * :children_crossing: UPDATE: tentative repro #731 * :children_crossing: NEW: e2e tests for union-result * use inline snapshots for artifact tes... — committed to HoudiniGraphql/houdini by AlecAivazis 2 years ago
Improve support for unions and interfaces (#747) * :children_crossing: UPDATE: tentative repro #731 * :children_crossing: NEW: e2e tests for union-result * use inline snapshots for artifact tes... — committed to gterras/houdini by AlecAivazis 2 years ago

Most upvoted comments

Hi @AlecAivazis, I thank you and @jycouet for this project. I also didn’t have time to look deeper into this bug, but whenever I can I want to help improve Houdini 🚀🚀

524c on Nov 29, 2022

Okayyy finally got some time to pull down the reproduction and figure out what was going on. Thanks so much for putting that together.

The quick answer is that the unions in the query results are confusing the cache when its trying to read values. Message is not nullable so when its reading the values back from the cache, it sees a missing message and cascades the null value all the way up, destroying the previous value. This will be quite an involved fix but I’ll try to get it working in the next few days. Luckily, the fix I have in mind is also going to address #651 which makes me happy 😄. In the meantime, you can get things working better by removing the ! in the definition for the message field. It’s not a perfect fix since if the error state changes between requests, you’ll just run into the problem again but at least it’s something.

Thanks for being patient!

AlecAivazis on Nov 29, 2022

Just an update - #747 should fix this. We reproduced in our e2e test suite and everything is working 🎉

AlecAivazis on Dec 7, 2022

Thx for the repo with the repro 😉 It will be easier to look what is going on there as I was able to reproduce it locally 👍

Looking quickly at the schema & houdini’s artifacts, I think that it’s link to the return union with ... on ZodError, ... on Error. I will have a deeper look tomorrow.

Thx🙏

jycouet on Nov 27, 2022

Just before doing all this, do you have a +page.ts and what’s inside (if you have one)?

In this example, I made the report, I don’t have a +page.ts.

When I noticed this problem, I had, initially, a +page.ts to bring a content already loaded from the server.

524c on Nov 27, 2022

Maybe you can start with the branch I just created with a fork?

Yes I will. I noticed that the problem only occurs in a scenario where my graphql server responds with a union interface and then I need to use fragments to read these responses. I’ll soon do a PR on this branch. Thanks.

524c on Nov 27, 2022