Why Doesn't mutate return a value on Error or Promise Rejections?

[martinmckenna]

Hi sorry for all the GitHub issues, I'm having another issue that i'm finding I may need to create a workaround for

I have code the roughly looks like this:

export const $createThing = createMutatorStore<CreateStreamPayload>(({ data, invalidate }) => {
  invalidate('things')
  return createThing(data);
})

...

const { mutate } = useStore($createThing)

const handleAddNewThing = async () => {
    try {
         const response = await mutate({ hello: 'world' });
         console.log(response) // only exist if non promise rejection
    } catch (e) {
       // never gets here
       showToastNotification('something went wrong!')
    }
}


<button onClick={handleAddNewThing} />

Now the issue here is that the only real way to show my toast notification is listen to error and do it in a useEffect like so:

export const $createThing = createMutatorStore<CreateStreamPayload>(({ data, invalidate }) => {
  invalidate('things')
  return createThing(data);
})

...

const { mutate, error } = useStore($createThing)

const handleAddNewThing = async () => {
    const response = await mutate({ hello: 'world' });
    console.log(response)
}

useEffect(() => showToastNotification('something went wrong!'), [error])

...

<button onClick={handleAddNewThing} />

So my question is, why can't I just get access to the error at the time I call mutate? Even react themselves say this is not a good way of reacting to changes:

https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers

Essentially what would be awesome is an option that will return the error inside this catch block:

https://github.com/nanostores/query/blob/main/lib/main.ts#L365

[gtrak]

Hi, thanks for the awesome lib! I'm working on the same team as Martin.

I would expect there are a few reasonable choices for this.

1. Bubble a thrown exception
2. Return a value that wraps the exception
3. Allow us to pass an error callback.

Ideally, we get to specify something like that in the createMutatorStore init.

What do you think?

[dkzlv]

Will this work for you? Seems simple enough.

const { mutate } = useStore($createThing)

const handleAddNewThing = async () => {
    const response = await mutate({ hello: 'world' });
    if ($createThing.value.error) { /* show toast */ }
}

Since you don't need reactivity in the callback, and $createThing is a module variable, I don't think it can backfire in any way.

See the thing is that mutate's expected not to throw, ever, by design. It's too simple to forget adding error handling somewhere and suddenly have shitty UX.

I also very much dislike when things are overly complicated or have too many options. As an example of behavior I don't like: Apollo Client. If you destructure error from useMutation OR provide onError handler, mutate will never throw; if you do neither, it will throw. Although I understand the motivation, it seems too smart for my taste, I'd like the fetching lib to be dumb and simple.

---

On a broader topic, I'd suggest moving toast state to nanostores as well. If you have everything in one state manager, you'll see how simple it is to solve such issues. In this particular case I'd most probably introduce a global onError handler that just shows a toast for all mutations for a given context. Or, if global handler doesn't work for you for some reason, I'd at least move error handling from component and into the mutator itself (after all, mutator can theoretically be used in many places, therefore it's a good place to hold the error handling).

[gtrak]

That makes sense. I like this lib for its simplicity, I can read the impl in one sitting. I agree implicit throw vs not throw is a debugging headache.

I've seen libs just take that behavior as a parameter, whether to reraise, wrap the error, or do something else with it. I really loved using Async Monitors: https://ocaml.org/p/async_kernel/latest/doc/Async_kernel/Monitor/index.html#val-try_with

We'll try out what you recommend, though.

[martinmckenna]

hey @dkzlv this seems to work, but it seems like the MutatorStore isn't properly typed and doesn't think value exists on it

We'll go with this for now, but I think even having an onError option on the actual createMutatorStore call, much like how the option exists today as an option to nanoquery(), would be amazing

React query also exposes an option like this:

https://tanstack.com/query/latest/docs/framework/react/guides/mutations#mutation-side-effects

[dkzlv]

@martinmckenna Oh, both MutatorStore and FetcherStore merely extend the MapStore from nanostores, and it does have value property. It was always there in the runtime, but we only decided to make it a part of public interface a few versions back. Try updating the nanostores lib to latest version, it should be ok.

As for your suggestion, I'll make another issue for it, and will close this one for now. Feel free to ping if something doesn't work for you!