ignoreNotFound operator

[JanMalch]

The operator can mark a resource as optional, so that when the server responds with the 404 status, the operator will safely return undefined instead. All other errors will be rethrown.

[nilsmehlhorn]

Hey Jan, thank you so much for the pull request, it looks well done!

I'm just struggling a bit with the operator itself. I like the idea of encoding optional values but I don't think mapping a specific status to an undefined value is the best choice for two reasons:

1. There is undefined and then there's null, additionally a request might return either of them as it's body (afaik). They don't force the developer to deal with them - the operator might therefore be error prone when you don't have strict null checks, which is the default for CLI projects (again, afaik). I personally prefer some kind of Optional type like the one from fp-ts: https://gcanti.github.io/fp-ts/modules/Option.ts.html. But I think it's too much to include it as a dependency for now.
2. Shouldn't an observable that can't provide a value just not provide a value? So, instead of emitting undefined, the observable would instead complete

Eventually, I'd propose to introduce a more generic operator called recoverForCodes. It'd be just like throwForCodes but instead of providing another error, one would provide a status-specific fallback value. You would then be able to implement the behaviour of your operator like this:

this.http.get<User>('/users/123').pipe(
  recoverForCodes({
    404: () => undefined // or rather: of(undefined)
  })
)

Otherwise, I'd also be open for a version of your operator, that completes instead of emitting undefined. We should probably just name it more concise - how about ignoreNotFound()?

[JanMalch]

Thanks for the reply. While undefined is not a valid JSON response, angular probably returns it for an empty body?

While it's true that developers aren't always forced to handle it, they are immediately aware that undefined could happen and they in fact want it. It's a simple operator with a simple documentation. In fact I think recoverForCodes would be much harder to type correctly, wouldn't it?

I would also prefer undefined over no value for cases where you want to mergeMap or combineLatest (etc...) observables.
Also I think it's less surprising if the observable is emitting undefined instead of just completing, since most developers would expect an observable to always emit at least once. And also in probably 99% of all cases an observable's subscription only handles the next part, which means you would have to write more code again to handle the "not found" case and it's more complicated to figure out if the observable completed with or without previously emitting a value.

I agree that an Optional type would be nice but too much as a dependency. We can also rename the operator.

[nilsmehlhorn]

While undefined is not a valid JSON response, angular probably returns it for an empty body?

It seems as if Angular actually returns null for empty bodies. The Angular source is pretty keen on null, TypeScript favors undefined. Interestingly enough, the RxJS operator defaultIfEmpty provides null as a fallback for empty observables (see test).

In fact I think recoverForCodes would be much harder to type correctly, wouldn't it?

recoverForCodes might be harder to type for us (best case we infer a type union from the return types of each recovery function), yet it wouldn't force a specific type/behaviour on developers. They could decide whether the observable should recover with undefined via 404: () => of(undefined) or rather complete via 404: () => EMPTY.

I'd also argue, that without strict null checks, people assume any type to be potentially undefined, therefore I'd like to somehow work around T | undefined

I would also prefer undefined over no value for cases where you want to mergeMap or combineLatest (etc...) observables. [...]

I see your point, but you could replace mergeMap with concatMap - depending on the use-case. There isn't a direct replacement for combineLatest when the underyling observable completes without a value, but that makes sense to me. For a version of this operator to work with combineLatest you'd have to provide a fallback value. Something that's meant to be optional can't be combined either way without having a replacement for when it's not present.

And also in probably 99% of all cases an observable's subscription only handles the next part

Would you mind giving an example of the cases you'd use the operators for where a completion would be inconvenient? I'm not quite sure what you want to do with the undefined value, that you wouldn't rather do in e.g. finalize().

My current proposal: let's postpone recoverForCodes but make this operator complete upon 404. We can also extent the docs with this "gotcha" and advise people to use defaultIfEmpty when they still want to have an emission.

[JanMalch]

My current proposal

Sounds good to me. I have to admit I didn't know about defaultIfEmpty.

Would you mind giving an example of the cases you'd use the operators for where a completion would be inconvenient?

I was thinking about the *ngIf | async pattern, but that would make no difference since async will emit null anyways. So nvm; I can't really think of a common use case where this would be applicable.

---

recoverForCodes

I think the point of the operator is to force a specific behaviour. Word it as a "recommendation" and it doesn't sound so bad anymore. That would be the trade-off here if you want to write less code / more concise code.

I'm not sure how much of a benefit recoverForCodes would be. It would only save maybe 1-3 LoC in comparison to a switch statement which would also still be readable and easily understable.

I'm not sure if I would remember additional operators if they do not provide that much in terms of LoC or semantic, especially when I already know how to use catchError and switch.

people assume any type to be potentially undefined

This heavily depends on the developer's know-how and experience :)

work around T | undefined

Why exactly? If they expect things to be undefined at any time they don't really have to care about the | undefined, but the strict-null-checks fellas get the correct typing.

[nilsmehlhorn]

Can you still make a use-case example for the operator being added anyway? I'm not quite sure about its use-cases - irrespective of how it behaves. I don't see where ignoring a 404 error is different from letting it bubble through in an actual application. If you can't recover why keep on going?

Either way, I'd rename it to ignoreNotFound() in compliance with ignoreElements

[JanMalch]

For example on an overview page for a user, where you want to show his car and his house. Your application has endpoints like /user/123/car and /user/123/house. Now you always fire these requests and they might return 404 as a REST-API should. Which would mean that the user doesn't have a car or a house. But that can be plausible within your application. It's not an error in the context of domain logic. So then you can do

this.userHouse$ = this.http.get<User>('/users/123/house').pipe(ignoreNotFound());

<app-house *ngIf="userHouse$ | async as houseData; else noHouse" [house]="houseData"></app-house>
<ng-template #noHouse><p>No house found</p></ng-template>

I have used such an operator in a big application and it was really useful.

[nilsmehlhorn]

Can you name the file ignore-not-found.ts (snake case) and the test file correspondingly? Right now it's optional.spec.ts instead of ignore-not-found.spec.ts

I'll. merge after these changes