redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.31k stars 995 forks source link

[Bug]: If an error is raised as part of the useMutation hook then the error message renders improperly as a FormError #6036

Open dthyresson opened 2 years ago

dthyresson commented 2 years ago

What's not working?

See https://github.com/redwoodjs/redwood/issues/5802

If an error is raised as part of a mutation that is not a GraphQL or Network error returned by the graphql-server as part of handling the operation then the error message renders as a FormError.

This message should not leak code messages about the error or should not be part of the FormError but instead some other page error rendering (maybe the cell error component? Although the mutation succeeds).

Need to determine the best rendering approach for these error types.

How do we reproduce the bug?

export const Success = ({ post }: CellSuccessProps<EditPostById>) => {
  const [updatePost, { loading, error }] = useMutation(UPDATE_POST_MUTATION, {
    onCompleted: () => {
      toast.success('Post updated2')
      null.f() // <--- boom
      navigate(routes.posts())
    },
    onError: (error) => {
      toast.error(error.message)
    },
  })

Or more likely,

export const Success = ({ post }: CellSuccessProps<EditPostById>) => {
  const [updatePost, { loading, error }] = useMutation(UPDATE_POST_MUTATION, {
    onCompleted: () => {
      toast.success('Post updated')

      navigate(routes.goToAMissingRoute)) // <--- boom because of a bad route
    },
    onError: (error) => {
      toast.error(error.message)
    },
  })

because of a bad check in the collection of graphql errors.

What's your environment? (If it applies)

No response

Are you interested in working on this?

Tobbe commented 2 years ago

As DT said, an error in onCompleted will render as a form error. But, IMHO, form errors are more like (server-side) validation errors, or a failure to create or update a resource or something like that. When the mutation has completed successfully on the server side it's no longer a form error (again, in my opinion).

Personally I'd probably expect this kind of error to be handled by my own ErrorBoundry or perhaps a try/catch inside onCompleted and then probably showing some kind of error flash message or printing a generic error message on the page somewhere.

Happy to discuss if someone doesn't agree 🙂