query icon indicating copy to clipboard operation
query copied to clipboard

Why Is Angular Still Throwing an Error Even Though It’s Caught in `onError`?

Open jon9090 opened this issue 10 months ago • 6 comments

Which @ngneat/query-* package(s) are the source of the bug?

query

Is this a regression?

No

Description

In my mutation function, I simulate an error:

mutationFn: () => {
  return of({ data: { status: 500 } }).pipe(
    tap(() => {
      console.log('in tap error');
      throw new Error('bla');
    })
  );
}

The error is caught in the onError function, but Angular still throws it, and nothing seems to catch it.
Check the console.

Is this expected behavior, or is it a bug?

[StackBlitz example](https://stackblitz.com/edit/stackblitz-starters-vzvvph3i)

import { Component } from '@angular/core';
import { bootstrapApplication } from '@angular/platform-browser';

import { of, tap, catchError, throwError, map } from 'rxjs';
import { injectMutation, injectQuery } from '@ngneat/query';
import { CommonModule } from '@angular/common';

console.clear();

@Component({
  selector: 'app-root',
  imports: [CommonModule],
  template: `
   in app
  `,
})
export class App {
  #query = injectQuery();

  #add = injectMutation();

  add = this.#add({
    mutationFn: () => {
      return of({ data: { status: 500 } }).pipe(
        tap(() => {
          console.log('in tap error');
          throw new Error('bla');
        })
      );
    },
    onSuccess: () => {
      console.log('onSuccess');
    },
    onError: () => {
      console.log('in error');
    },
  }).result;

  ngOnInit() {
    this.add().mutate({});
  }
}

bootstrapApplication(App);

jon9090 avatar Feb 14 '25 17:02 jon9090

Error Boundaries Error Boundaries are a general concept in React to catch runtime errors that happen during rendering, which allows us to react (pun intended) properly to them and display a fallback UI instead. This is nice because we can wrap our components in Error Boundaries at any granularity we want, so that the rest of the UI will be unaffected by that error. One thing that Error Boundaries cannot do is catch asynchronous errors, because those do not occur during rendering. So to make Error Boundaries work in React Query, the library internally catches the error for you and re-throws it in the next render cycle so that the Error Boundary can pick it up.

taken from: React Query Error Handling

So if im correct on that, it means that the next render cycle (aka. the framework/angular) needs to pick these up with zone or a global error handler. Luckily the zone picks them up for us which is clearly stated by the stackblitz you've provided.

Image

onError itself wont just catch the error, it will be rethrown to give the mutate function, the mutation itself and angular the chance to catch them via either these callbacks (to execute some sideeffects, like optimistic updates for example) or a error handler (in angular). If you for example would want to use Sentry to catch these errors, just catching them inside the mutation would be no good, since you would need to add onError to any mutation you create and manually send them up. Where on the other hand, catching them via a global error handler is easier.

luii avatar Feb 15 '25 06:02 luii

@luii , I've extended the example without using signals. The code remains the same:

remove = this.#add({
  mutationFn: () => {
    return of({ data: { status: 500 } }).pipe(
      tap(() => {
        console.log('in tap error');
        throw new Error('bla');
      })
    );
  },
  onSuccess: () => {
    console.log('onSuccess');
  },
  onError: () => {
    console.log('in error');
  },
});

When calling this.remove.mutate({});, the error doesn't propagate outside the function, which is good. I don’t want Angular's global error handler to catch all errors—I prefer handling them in the onError method.

[StackBlitz Example](https://stackblitz.com/edit/stackblitz-starters-vzvvph3i?file=src%2Fmain.ts)

In React Query, the throwOnError property controls this behavior:

remove = this.#add({
  onError: ...
  throwOnError: false,
});

By default, it's false, so exceptions aren’t thrown. If set to true, React Query throws the error regardless.

IMHO, something is missing in the Signal-based query implementation for handling errors correctly.

If I do this:

this.add().mutate({}).catchError(e => e);

It solves the problem, but it feels weird. Also, I can't do it directly in the template:

(click)=" add().mutate({}).catch(e => e)"

This forces me to write a wrapper function for every call when using signals, which is bad practice (just to wrap a function).

Is there a solution for this in @ngneat/query?

jon9090 avatar Feb 15 '25 14:02 jon9090

I prefer handling them in the onError method.

I dont think onError is built for handling errors as a try catch would, its just a callback to inform the some other part of your code to for example rollback your ui changes or show a toast. The error handling itself should either come from the underlying pipe (where you use of({ data: { status: 500 }})) which can be catched by a catchError rxjs operator (order is important here, since some operators are downflowing), or, like you did, on the outside of the mutation with catch.

IMHO, something is missing in the Signal-based query implementation for handling errors correctly.

Yes i agree, error handling is lacking in the signal based api, but not exclusively in ngneat-query, its a general weakness of it. Netanel also wrote a blog post about it how it could/can be achieved. For this kind of reason its good to use rxjs as long as possible, they have that kind of notification system, that netanel wrote about already implemented and strengthend. For me personally the mix of both is the key, handling requests as long as possible with rxjs and on the last mile where your observable would connect to the template, i use signals and computations.

It solves the problem, but it feels weird. Also, I can't do it directly in the template:

For that case you could use a wrapper function in the template, hiding such implementation details from your template is generally a good idea, keeps it nice and clean and good to read.

Is there a solution for this in @ngneat/query?

ATM no, there is not a solution to it.

luii avatar Feb 15 '25 16:02 luii

It appears that @tanstack/angular-query has a built-in mechanism to prevent errors from propagating.

allPosts$ = () =>
    this.#http
      .get<Array<Post>>('https://jsonplaceholder.typicode.com/posts')
      .pipe(
        tap(() => {
          throw new Error('some');
        })
      );

Link to example on StackBlitz

jon9090 avatar Feb 22 '25 07:02 jon9090

It appears that @tanstack/angular-query has a built-in mechanism to prevent errors from propagating.

allPosts$ = () => this.#http .get<Array<Post>>('https://jsonplaceholder.typicode.com/posts') .pipe( tap(() => { throw new Error('some'); }) );

Link to example on StackBlitz

I don't see how your reply is related to it right now, maybe i'm missing something here?

luii avatar Feb 24 '25 12:02 luii