The syntax you suggest seems reasonable. I’m less tied to how it’s written than where, because I generally find bugs caused by interactions of diffusely written code to be simultaneously easy to make and hard to diagnose. This is partially why I love GC and DI so much - manually managing object lifecycle has caused me no end of frustration over the years.
Makes sense. I’ll add that extra layer of defense to my Subjects.
I recently encountered a particularly nasty bug related to this concept.
export class AuthService {
authtoken = new ReplaySubject<string>(1);
// imagine other things calling `next` to set authorization tokens
}
export class AuthHttpInterceptor implements HttpInterceptor {
constructor(private _auth: AuthService) {
}
intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
return this._auth.authtoken
.pipe(
mergeMap((jwt) => {
let areq = req.clone({setHeaders: {Authorization: 'Bearer ' + jwt}});
return next.handle(areq);
}));
}
}
export class OtherService {
constructor(private _http: HttpClient) {}
thingy(): Observable<Thingy> {
return forkJoin(this._http.get('/thing1'), this._http.get('/thing2').pipe(
mergeMap(things => frobulate(things))
);
}
}
export class Page {
thingy: Thingy;
constructor(private _svc: OtherService) {}
fetchThingy(): void {
this._svc.thingy().subscribe(thingy => this.thingy = thingy);
}
}
This is also related to the “diffuse coding bug” topic, because it’s the interceptor that unwittingly causes the bug. OtherService.thingy() will never emit.
After several hours of head-scratching and narrowing, I realized that since the authtoken ReplaySubject never completes, neither will any of the intercepted HTTP requests (although they will emit, which means I didn’t notice the interceptor bug when just simply subscribing to requests, only when attempting to forkJoin multiple ones).
A solution is to add first() to the beginning of the interceptor’s pipe.
Why first() as opposed to take(1) ?
To emit an error if necessary?
take(1) isn’t good enough. My favorite example of this problem is
obs.filter(x => x!==null).take(1)
obs never completes if it never emits a non-null value, despite the presence of take(1). So it’s potentially a long term memory leak that looks super safe.
Pretty much. Imagine we have a “continue as guest” option that gives up on being authorized, so the token stream could complete without ever having sent anything. I think I would prefer any pending authorization-needed intercepted requests that are waiting on tokens to recognize that situation as “not normal”.
Definitely good to know. Was not aware of that.
The app i’m working on right now just so happens to have a isVisitor status that I am observing, so that’s a very helpful point for me.
Thanks @rapropos
For anybody who stumbles across this conversation later, I have recently become aware of ngx-take-until-destroy, which automates part of this tripwire idiom.
2 Likes
In my most recent apps I use Ngrx and manage to do without a manual subsribe, just using async with ngIf and ngFor. If there is a neeed to fill a field for the component (specific Ui state like a form) I use tap on the pipe.
So all in the angular template with local variables. And as such sometimes async one observable stream twice in the template (eg. title and content using ngIf)
Saves quite some lines and I assume more performing then doing all stuff manually…
Any considerations?