Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request for first et al: Configurable empty errors. #7330

Open
benlesh opened this issue Sep 6, 2023 · 0 comments
Open

Feature request for first et al: Configurable empty errors. #7330

benlesh opened this issue Sep 6, 2023 · 0 comments

Comments

@benlesh
Copy link
Member

benlesh commented Sep 6, 2023

It's come up more than once in recent days where I've run across code or confused developers that didn't know where an EmptyError was originating from.

Basically one of the problems is this:

source$.pipe(
  /* ... operators ... */
  first(somePredicate),
  /* ... operators ... */
  first(somePredicate),
  /* ... operators ... */
  catchError(err => {
    if (err instanceof EmptyError) {
      // who did this?
    }
  })
)

Current Workarounds

You can always leverage the defaultValue argument, but if you want to use the error channel, you'd have to do something like this catching and rethrowing right after:

source$.pipe(
  /* ... operators ... */
  first(somePredicate),
  catchError(err => {
    if (err instanceof EmptyError) {
      throw new CustomError1();
    }
    throw err;
  }),
  /* ... operators ... */
  first(somePredicate),
  catchError(err => {
    if (err instanceof EmptyError) {
      throw new CustomError2();
    }
    throw err;
  })
  /* ... operators ... */
)

This can of course be made into something reusable something like so:

export function firstOtherwise<T, O>(
  {
    find,
    otherwise
  } : {
    find: (value: T, index: number) => boolean,
    otherwise: () => O
  }
): OperatorFunction<T, T> {
  return pipe(
    first(find),
    catchError(err => {
      if (error instanceof EmptyError) {
        return of(otherwise())
      }
      throw err;
    })
  )
}

Perhaps a better workaround for some people is filter, take and throwIfEmpty:

source$.pipe(
  filter(predicate),
  take(1),
  throwIfEmpty(() => new CustomError1()),
)

Proposed Solution 1: Configuration

Neither of the above are great. Sadly, the best solution to this will require a breaking change we can't really deprecate cleanly (but we can deprecate in a dirty way):

source$.pipe(
  /* ... operators ... */
  first(somePredicate, { emptyError: () => new CustomError1() }),
  /* ... operators ... */
  first(somePredicate, { emptyError: () => new CustomError2() }),
  /* ... operators ... */
)

The problem with this solution is the second argument is currently the defaultValue, has a similar use case. We could examine the default value like typeof defaultValue === 'object' && 'emptyError' in defaultValue and it would probably be fine in 99.999999% of cases, but it's not bulletproof. That object would also have to support a defaultValue property, so we'd need to check for that too. For at least a whole major release.

Proposed Solution 2: Function

In this one we would examine the second argument, and if it's a function, we'd call it to get the default value. If it throws it emits the error (obviously).

source$.pipe(
  /* ... operators ... */
  first(somePredicate, () => 'Some default value'), // default value
  /* ... operators ... */
  first(somePredicate, () => { throw new CustomError2() }), // custom empty error
  /* ... operators ... */
)

The problem here is I think collisions with existing code and regressions are more likely than the other one. It's also not as readable.

Proposed Solution 3: Combination of the above

In this one we have a configuration/options object with a defaultValue, valueNotFound, onEmpty, or getDefault (bike sheddable) property on it that would have a function that works as solution 2 above does.

source$.pipe(
  /* ... operators ... */
  first(somePredicate, {
    whenValueNotFound: () => 'Some default value'
  }), // default value
  /* ... operators ... */
  first(somePredicate, {
    valueNotFound: () => { throw new CustomError2() }
  }), // custom empty error
  /* ... operators ... */
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant