TheEdoRan / next-safe-action

Type safe and validated Server Actions in your Next.js (App Router) project.
https://next.next-safe-action.dev
BSD 3-Clause "New" or "Revised" License
1.38k stars 26 forks source link

[FEATURE] Support throwing server errors in addition to returning them #79

Closed VladyslavL closed 2 months ago

VladyslavL commented 2 months ago

Is there an existing issue for this?

Library version (optional)

6.1.0

Ask a question

First of all, thanks for the package. It is awesome!

React Query requires a new Error trowing to fire onError callback and properly handle errors. NSA returns an object a data object with serverError entry inside with type string | undefined. Is there a way to throw a new Error instead of handling errors inside the onSuccess callback?

Thanks

const { mutate, error } = useMutation({
    mutationFn: (sum: string) => postPaymentAction({ sum }),
    onSuccess(data) {
      // data here
    },
    onError(error) {
      // NEVER WILL BE FIRED

      toast.error(error.message);
    },
  });

Additional context

No response

TheEdoRan commented 2 months ago

Hi @VladyslavL, I've never used next-safe-action with React Query, since next-safe-action provides onExecute, onSuccess, onError and onSettled callbacks too, via useAction/useOptimisticAction hooks, and when using the library hooks, errors are correctly handled by the onError callback.

I guess I can add an init option to createSafeActionClient called throwOnServerError that changes the behavior to throw errors when one occurs on the server, instead of returning it as a string.

Just out of curiosity, why are you relying on useMutation hook from React Query, instead of using the useAction hook from next-safe-action?

VladyslavL commented 2 months ago

@TheEdoRan , this is good question actually :) I went so deep with the errors handling problem that hadn't even thought about replacing useMutation with useAction...

However, it would be great to have the throwOnServerError option as well.

Thanks for the reply!

TheEdoRan commented 2 months ago

However, it would be great to have the throwOnServerError option as well.

Yeah, I agree, it's useful in some other cases too.

Thanks for the reply!

You're welcome!

I'll leave this issue open until the PR with throwOnServerError will be merged.

TheEdoRan commented 2 months ago

I thought a little bit more about this issue, and actually there's already a way to throw server errors, instead of returning them.

Using handleReturnedServerError, you can customize server error messages returned to the client, when middleware/server code function execution fails.

Well, rethrowing the error inside this function will actually throw it, and you're in charge of catching it in this case. Furthermore, you can rethrow all errors or restrict the throwing to specific custom error classes, which offers a granular control of server errors behavior. This wouldn't be possible when using a global throwOnServerError option.

Here are two examples.

1. Rethrowing all errors

In this example, if any error occurs on the server, it will be rethrown:

import { createSafeActionClient } from "next-safe-action";

export const action = createSafeActionClient({
  handleReturnedServerError(e) {
    throw e;
  },
});

2. Rethrowing specific errors

In this example, we'll instead rethrow just ActionErrors, while keeping the default server error message for all other kinds of errors:

import { DEFAULT_SERVER_ERROR, createSafeActionClient } from "next-safe-action";

export class ActionError extends Error {}

export const action = createSafeActionClient({
  handleReturnedServerError(e) {
    if (e instanceof ActionError) {
      throw e;
    }

    return DEFAULT_SERVER_ERROR;
  },
});

Please let me know if this solves your issue. Thanks.

VladyslavL commented 2 months ago

Thanks @TheEdoRan! It works with React Query, but now onError from useAction does nothing.

To give you more information about my setup:

I use next-safe-action for all requests, GET,POST,PATCH to make my app more secure and hide API calls (why not, right? :D ). In some cases, I use React Query, which makes polling very simple, on some pages, but I also use useAction on other pages to make my code simpler and lighter.

However, I can just use React Query on all pages in case of throwing an error on createSafeActionClient and this is great. Thanks again! :)

TheEdoRan commented 2 months ago

but now onError from useAction does nothing.

Well, the problem is that even if I add a global throwOnServerError option, the issue wouldn't go away. It would be even worse, since you'd lose control over custom error classes behavior with that option.

You're experiencing the expected behavior, because if an error gets caught by next-safe-action, you can access it via serverError and onError callback with hooks. If an error doesn't get caught, though, it bubbles up to the component level. If you pass the action to the useMutation hook from React Query, the error will be correctly handled by it.

I use React Query, which makes polling very simple

In the future we can also improve useAction/useOptimisticAction hooks, so if you want to suggest a feature to implement, feel free to open a new issue about a specific functionality you want to see in this library.

However, there are already two ways to achieve the behavior you want here:

1. Use a separate client for React Query actions

If you know you're going to use an action with React Query, you can just define a separate safe action client for this use case. Let's call it queryAction. This client will rethrow all errors, so this works great with React Query. If you instead want to use the useAction hook from this library, you can switch to the "regular" action client to define the action (queryAction() -> action()):

// This is used to define actions used without React Query.
export const action = createSafeActionClient();

// This is used to define actions used with React Query.
export const queryAction = createSafeActionClient({
  handleReturnedServerError(e) {
    throw e;
  },
})

2. Use a single client and catch API errors

You can also achieve this by using a single client. To do this you have to define a custom error class, pass the handleReturnedServerError function to createSafeActionClient to rethrow errors of that class, and catch API errors that occur during action execution. Here's how to do it:

// Safe client file

// Custom error class
export class APIError extends Error {}

export const action = createSafeActionClient({
  handleReturnedServerError(e) {
    // Rethrow API errors (then handled by React Query).
    if (e instanceof APIError) {
      throw e;
    }

    // Return the default server error message for all the other errors.
    return DEFAULT_SERVER_ERROR;
  },
});

// Action file

"use server";

export const testAction = action(schema, async (input) => {
  // ...

  try {
    await apiCall();
  } catch (e) {
    // This will be rethrown by the `handleReturnedServerError` function.
    throw new APIError(e.message);
  }

  // ...
});

Thank you for bringing up this discussion, I'm going to close the issue now, since I explained a few ways to hopefully solve your problem in a somewhat clean way. If you still need help just leave a comment here, and I'll reopen the issue, if needed.

VladyslavL commented 2 months ago

Everything clear as possible! Thanks again @TheEdoRan!