Handle errors in updates

[lukasluecke]

I think there should be a clean way to handle mutation errors inside the update functions. Right now we only have access to result: Data. We should either provide the errors as well, or just not call the update if there's an error?

[JoviDeCroock]

I personally wouldn't make errors part of our cache, I would, however, support the notion of not instantiating a write when our mutation has returned erroneously

[lukasluecke]

The problem I have right now is that the data I get inside my update function doesn't match the expected shape / type, if there has been an error. I can just check the data, and skip if it's not there - but that didn't feel like the cleanest solution 🙈

I'm trying to think about a possible use case for having access to the errors inside the update, but don't have one right now (at least not one that could be solved another way). So just skipping if there's an error should be fine.

Do you know how that would play into optimistic updates? They would have to be reverted on error, even if we don't write. (I just started rewriting our app with this cache, so not using optimistic updates yet 🙂)

[JoviDeCroock]

It won't interfere because before write we delete potential optimistic keys https://github.com/FormidableLabs/urql-exchange-graphcache/blob/master/src/exchange.ts#L201

Should be thoroughly tested though ^^

[kitten]

A couple of thoughts here,

If the errors indeed lead to null data we shouldn't trigger the update function. I'll double check whether that's indeed the case 👍

Optimistic updates should be unaffected, as said, since we're wiping all optimistic changes completely once the real result comes in, no matter what state it actually arrives in ✨

Lastly, I'm not sure whether updates should be concerned with errors. If your data has optional fields, then it stands to reason that an error may occur on any of these fields, which would leave another part of the data alone.

In such a case it'd still make sense to apply updates in a lot of cases and I'd argue it's possibly cleaner to check whether the part of the data that you care about is actually there.

In most cases you'd for instance write a "Post" even if an error occurred and you don't have its optional "author" field, to give a crude example.

[lukasluecke]

I think that as long as the types that will be added for #75 correctly reflect the possible values (i.e. nullability), and/or we don't call the update function for completely null data, things would probably be fine for now. It's probably better to discuss this further once the type thing is solved, because then we'll see where the limits of that are.