ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.01k stars 1.97k forks source link

@ngrx/store, @ngrx/data Detected unserializable action at "payload.data.error" (DataServiceError) #1993

Closed KrzysztofKarol closed 4 years ago

KrzysztofKarol commented 5 years ago

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/data-service-error?file=src%2Fapp%2Fpost-data.service.ts

Expected behavior:

DataServiceError is somehow serializable or I can catch it and serialize on my own

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

@angular/common: 8.1.0 @angular/compiler: 8.1.0 @angular/core: 8.1.0 @angular/forms: 8.1.0 @angular/platform-browser: 8.1.0 @angular/platform-browser-dynamic: 8.1.0 @angular/router: 8.1.0 @ngrx/data: 8.0.1 @ngrx/effects: 8.0.1 @ngrx/entity: 8.0.1 @ngrx/store: 8.0.1 @ngrx/store-devtools: 8.0.1

Other information:

It's thrown here: https://github.com/ngrx/platform/blob/c59c21150293ad2c9ee5b03782b4fda7ce9f15c7/modules/store/src/meta-reducers/serialization_reducer.ts#L84

Should it be catched in custom data service? https://stackblitz.com/edit/data-service-error-workaround?file=src/app/post-data.service.ts

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request) [ ] No

Understanding the flow is not ease but I'm beginning to see the light.

peimelo commented 5 years ago

Hi, I haven't used @ngrx/data yet, but when I used @ngrx/effects I had the same problem:

loadWeights$ = createEffect(() =>
  this.actions$.pipe(
    ofType(WeightsActions.loadWeights),
    mergeMap(({ pageIndex }) =>
      this.weightsService.getAll(pageIndex).pipe(
        map(weightResponse =>
          WeightsActions.loadWeightsSuccess({ weightResponse })
        ),
        catchError(error => {
          return of(WeightsActions.loadWeightsFailure({ error })); <= ERROR
        })
      )
    )
  )
);

ERROR Error: Detected unserializable action at "error"

I resolved by returning the error.message instead of error:

loadWeights$ = createEffect(() =>
  this.actions$.pipe(
    ofType(WeightsActions.loadWeights),
    mergeMap(({ pageIndex }) =>
      this.weightsService.getAll(pageIndex).pipe(
        map(weightResponse =>
          WeightsActions.loadWeightsSuccess({ weightResponse })
        ),
        catchError(error => {
          return of(WeightsActions.loadWeightsFailure({ error: error.message })); <= SOLVED
        })
      )
    )
  )
);
KrzysztofKarol commented 5 years ago

Thank you, @peimelo! I'll try it today.

KrzysztofKarol commented 5 years ago

Yes, somethink like this (https://stackblitz.com/edit/data-service-error-workaround?file=src%2Fapp%2Fpost-data.service.ts) can work but it's better to handle it in @ngrx

vicki2576 commented 5 years ago

Thank you

A-"74=HiwB8-^=d

On Tue, Jul 23, 2019, 6:15 PM Krzysztof Karol notifications@github.com wrote:

Thank you, @peimelo https://github.com/peimelo! I'll try it today.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ngrx/platform/issues/1993?email_source=notifications&email_token=AMBAQFLWZHLIOGMU3LAILO3QA6GHZA5CNFSM4H5GZLW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UWEWQ#issuecomment-514417242, or mute the thread https://github.com/notifications/unsubscribe-auth/AMBAQFO3X6UXP6OZ4XTEREDQA6GHZANCNFSM4H5GZLWQ .

vicki2576 commented 5 years ago

Thank you

A-"74=HiwB8-^=d

On Tue, Jul 23, 2019, 5:42 PM Paulo Melo notifications@github.com wrote:

Hi, I haven't used @ngrx/data yet, but when I used @ngrx/effects I had the same problem:

loadWeights$ = createEffect(() => this.actions$.pipe( ofType(WeightsActions.loadWeights), mergeMap(({ pageIndex }) => this.weightsService.getAll(pageIndex).pipe( map(weightResponse => WeightsActions.loadWeightsSuccess({ weightResponse }) ), catchError(error => { return of(WeightsActions.loadWeightsFailure({ error })); <= ERROR }) ) ) ) );

ERROR Error: Detected unserializable action at "error"

I resolved by returning the error.message instead of error:

loadWeights$ = createEffect(() => this.actions$.pipe( ofType(WeightsActions.loadWeights), mergeMap(({ pageIndex }) => this.weightsService.getAll(pageIndex).pipe( map(weightResponse => WeightsActions.loadWeightsSuccess({ weightResponse }) ), catchError(error => { return of(WeightsActions.loadWeightsFailure({ error: error.message })); <= SOLVED }) ) ) ) );

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ngrx/platform/issues/1993?email_source=notifications&email_token=AMBAQFIU6NMID434QQPEUC3QA6CPHA5CNFSM4H5GZLW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UUOPQ#issuecomment-514410302, or mute the thread https://github.com/notifications/unsubscribe-auth/AMBAQFI2D4RRSLIO53544A3QA6CPHANCNFSM4H5GZLWQ .

KrzysztofKarol commented 5 years ago

Setting runtimeChecks to

strictActionSerializability: false,

"solves" the problem.

andrsbrbs commented 4 years ago

Yes, somethink like this (https://stackblitz.com/edit/data-service-error-workaround?file=src%2Fapp%2Fpost-data.service.ts) can work but it's better to handle it in @ngrx

How do I handle it via NgRx? I'm using @ngrx/data with a custom API url, and got the same error until I added the catchError operator to the service's request.

jemerald commented 4 years ago

Setting runtimeChecks to

strictActionSerializability: false,

"solves" the problem.

This only hides the problem, which is to do with @ngrx/data trying to pass the HttpErrorResponse object in the action payload. The HttpErrorResponse object contains function definitions that cannot be serialized.

@ngrx/data should either map the HttpErrorResponse to simple object, or provide an override for HTTP error response mapping, or both.

luchillo17 commented 4 years ago

@peimelo I'm using @ngrx/data and getting control of the error is harder here, though I do understand the issue is with the error itself not being serializable at some point.

I was going to override the DefaultPersistenceResultHandler to cast the error into a serializable object to work around this issue, however, it seems like it's part of the architecture to transform the error (which I'm throwing in an interceptor as a string) to a non-serializable DataServiceError.

https://github.com/ngrx/platform/blob/master/modules/data/src/dataservices/persistence-result-handler.service.ts#L51-L71

If that's the case, can't find where am I supposed to override, looks to me that transforming into DataServiceError was intentional and it's a bug where it wasn't transformed back into serializable error somewhere else.

brandonroberts commented 4 years ago

We've added a feature to ignore runtime checks for actions that originate from NgRx libraries that should help with this. See the PR here https://github.com/ngrx/platform/pull/2351. You can test it out with the 9.0.0-beta.0 release

luchillo17 commented 4 years ago

I see, so it will be fixed in next release, any ETA on it? will it be for angular 9 up?

brandonroberts commented 4 years ago

Soon-ish 😉. It will be for Angular 9 up, as that's our official support policy on major releases.

brandonroberts commented 4 years ago

Closing as resolved

luchillo17 commented 4 years ago

@brandonroberts I've just updated to NG9 and the error keeps happening, wasn't the fix deployed with version 9.0.0? Maybe I'm missing something, I'm throwing an error from an Http interceptor, and I throw an error string, but even after the update it still happens.

AdditionAddict commented 4 years ago

@luchillo17 please can you produce a reproduction in stackblitz or a github link

luchillo17 commented 4 years ago

Is there a base StackBlitz link from where to start?

zendu commented 4 years ago

I am using ngrx 9.1.0 and I see this error.

ivdim1992 commented 4 years ago

same here.. let explain what happened

  1. export const signInUserFailure = createAction( '[Authentication Module] Sign In User Failure', props<{ error: HttpErrorResponse }>() ); -> that is my error handler action

  2. public readonly signInUser$ = createEffect(() => this.actions$.pipe( ofType(AuthActions.signInUser), exhaustMap(({ data }) => this.authService.signIn(data).pipe( map((user) => AuthActions.signInUserSuccess({ user })), catchError((error) => of(AuthActions.signInUserFailure({ error }))) ) ) ) ); --> that's my effect -- here on the sign in I was receiving this specific error, when I changed the action to receive props<{error:{message: string}}>() --> the error disappeared...

  3. When I signed in and after that log out and return to the previous code with the previous action the error did not show again.. which is sooo strange can not understand what actually caused it

SanteriMagister commented 3 years ago

This still happens to me in the current version on NgRx Data. Has anyone found a workaround?

cbejensen commented 3 years ago

This error is still happening to me as well.

Here's a bare-bones reproduction: https://stackblitz.com/edit/angular-ivy-sxycrg?file=src/app/app.module.ts

krema commented 1 year ago

This still happens to me in the current version on NgRx Data. Has anyone found a workaround?

{ error: JSON.parse(JSON.stringify(error)) })