Skip to content

Memoized selectors: Inconsistent behavior when projection fails #2100

Closed
@REPLicated

Description

Description

The current behavior of memoization as implemented in defaultMemoize
can be very surprising in the case of a failed evaluation of the projection.

An error may be thrown in a projector either due to a bug in the projector logic itself (unanticipated cases when the projector is very complex) or when a precondition for the projector is not fulfilled (i.e. inconsistent store state due to a bug elsewhere).

Let selector be any memoizing selector, GS a "good state" from which R is selected, and FS a "bad state" for which the projection fails. The current behavior for three consecutive calls to the selector is (pseudocode):

selector(GS) => returns R
selector(FS) => throws
selector(FS) => returns R

I would expect the third call to the selector to throw as well. There seems to be no good argument why R should suddenly be selected from FS.
Therefore I suggest a fix to the memoization logic such that:

selector(GS) => returns R
selector(FS) => throws
selector(FS) => throws

Minimal reproduction of the bug/regression with instructions:

The following minimal (currently failing) testcase reproduces the issue:

      const firstState = { ok: 'a' };
      const secondState = { faulty: 'b' };
      const fail = () => { throw new Error(); };
      const selector = createSelector(
        identity,
        (s: any) => s.ok ? s.ok : fail()
      );

      selector(firstState);

      expect(() => selector(secondState)).toThrow(new Error());
      expect(() => selector(secondState)).toThrow(new Error()); // fails here, returns 'a'

Expected behavior:

As described above, I would expect more consistent behavior when the selector is triggered a second time with the same "faulty" state.

In simple scenarios, the current behavior may often be overlooked because the selector is usually not evaluated again with the same state (due to distinctUntilChanged in the store). However, if the selector is nested and only a part of the state changes it may happen of course. Further, if a select fails due to an error in a selector, the observable usually errors out. But when chained with operators like retry, the current behavior can obfuscate problems and may be very hard to debug.

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

Current NgRx master

Other information:

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

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions