Visitar URL original
fix(http): prevent interceptor signal reads from leaking into calling… · angular/angular@6d043f8 · GitHub
Skip to content

Commit 6d043f8

Browse files
sonukapoormattrbeck
sonukapoor
authored and
mattrbeck
committed
fix(http): prevent interceptor signal reads from leaking into calling reactive contexts
When `HttpClient` is called from within an `effect()` or other reactive context, any signal reads performed inside HTTP interceptors were inadvertently tracked by that context. This caused the effect to re-execute whenever those signals changed, regardless of whether the signal was semantically related to the HTTP call. The fix wraps the interceptor chain invocation in `untracked()` so that signal reads inside interceptors — both functional (`withInterceptors`) and class-based (`withInterceptorsFromDi`) — are invisible to the calling reactive context. This matches the precedent set by the resource API, which also wraps its loader in `untracked()` for the same reason. Fixes #58682
1 parent ae0ec73 commit 6d043f8

2 files changed

Lines changed: 106 additions & 6 deletions

File tree

‎packages/common/http/src/backend.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@
99
import {Observable} from 'rxjs';
1010

1111
import {
12-
ɵConsole as Console,
1312
EnvironmentInjector,
14-
ɵformatRuntimeError as formatRuntimeError,
1513
inject,
1614
Injectable,
15+
untracked,
16+
ɵConsole as Console,
17+
ɵformatRuntimeError as formatRuntimeError,
1718
PendingTasks,
1819
} from '@angular/core';
1920
import {finalize} from 'rxjs/operators';
@@ -116,14 +117,15 @@ export class HttpInterceptorHandler implements HttpHandler {
116117
);
117118
}
118119

120+
const chain = this.chain;
119121
if (this.contributeToStability) {
120122
const removeTask = this.pendingTasks.add();
121-
return this.chain(initialRequest, (downstreamRequest) =>
122-
this.backend.handle(downstreamRequest),
123+
return untracked(() =>
124+
chain(initialRequest, (downstreamRequest) => this.backend.handle(downstreamRequest)),
123125
).pipe(finalize(removeTask));
124126
} else {
125-
return this.chain(initialRequest, (downstreamRequest) =>
126-
this.backend.handle(downstreamRequest),
127+
return untracked(() =>
128+
chain(initialRequest, (downstreamRequest) => this.backend.handle(downstreamRequest)),
127129
);
128130
}
129131
}

‎packages/common/http/test/provider_spec.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@ import {HttpClientTestingModule, HttpTestingController, provideHttpClientTesting
2525
import {
2626
ApplicationRef,
2727
createEnvironmentInjector,
28+
effect,
2829
EnvironmentInjector,
2930
ErrorHandler,
3031
inject,
32+
Injector,
3133
InjectionToken,
3234
PLATFORM_ID,
3335
Provider,
36+
signal,
3437
} from '@angular/core';
3538
import {TestBed} from '@angular/core/testing';
3639
import {EMPTY, Observable, from} from 'rxjs';
@@ -671,3 +674,98 @@ const FAKE_JSONP_BACKEND_PROVIDER = {
671674
handle: (req: HttpRequest<never>) => EMPTY,
672675
},
673676
};
677+
678+
describe('HttpInterceptor signal tracking', () => {
679+
afterEach(() => {
680+
try {
681+
TestBed.inject(HttpTestingController).verify();
682+
} catch {}
683+
});
684+
685+
it('should not track signal reads in interceptors from a calling effect', () => {
686+
const interceptorSignal = signal(1);
687+
let effectRunCount = 0;
688+
689+
TestBed.configureTestingModule({
690+
providers: [
691+
provideHttpClient(
692+
withInterceptors([
693+
(req, next) => {
694+
// Reading a signal here must not be tracked by a calling reactive context.
695+
interceptorSignal();
696+
return next(req);
697+
},
698+
]),
699+
),
700+
provideHttpClientTesting(),
701+
],
702+
});
703+
704+
const http = TestBed.inject(HttpClient);
705+
const injector = TestBed.inject(Injector);
706+
const controller = TestBed.inject(HttpTestingController);
707+
708+
effect(
709+
() => {
710+
effectRunCount++;
711+
http.get('/test').subscribe();
712+
},
713+
{injector},
714+
);
715+
716+
// Run the initial effect.
717+
TestBed.tick();
718+
expect(effectRunCount).toBe(1);
719+
controller.expectOne('/test').flush('');
720+
721+
// Mutate the signal that was read inside the interceptor.
722+
interceptorSignal.set(2);
723+
TestBed.tick();
724+
725+
// The effect must NOT have re-run — interceptor signal reads are untracked.
726+
expect(effectRunCount).toBe(1);
727+
controller.verify();
728+
});
729+
730+
it('should not track signal reads in legacy class interceptors from a calling effect', () => {
731+
const interceptorSignal = signal(1);
732+
let effectRunCount = 0;
733+
734+
class SignalReadingInterceptor implements HttpInterceptor {
735+
intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
736+
interceptorSignal();
737+
return next.handle(req);
738+
}
739+
}
740+
741+
TestBed.configureTestingModule({
742+
providers: [
743+
provideHttpClient(withInterceptorsFromDi()),
744+
{provide: HTTP_INTERCEPTORS, useClass: SignalReadingInterceptor, multi: true},
745+
provideHttpClientTesting(),
746+
],
747+
});
748+
749+
const http = TestBed.inject(HttpClient);
750+
const injector = TestBed.inject(Injector);
751+
const controller = TestBed.inject(HttpTestingController);
752+
753+
effect(
754+
() => {
755+
effectRunCount++;
756+
http.get('/test').subscribe();
757+
},
758+
{injector},
759+
);
760+
761+
TestBed.tick();
762+
expect(effectRunCount).toBe(1);
763+
controller.expectOne('/test').flush('');
764+
765+
interceptorSignal.set(2);
766+
TestBed.tick();
767+
768+
expect(effectRunCount).toBe(1);
769+
controller.verify();
770+
});
771+
});

0 commit comments

Comments
 (0)