fix(http): Delay stabilization until next app synchronization (#60656)

atscott · atscott · commit 5795e03cdf25 · 2025-04-03T12:52:34.000-07:00
This commit updates the `HttpClient` internals to use the public `PendingTasks` API which delays stability until the next `ApplicationRef.tick` instead of causing the application to become stable synchronously. This is helpful to resolve unexpected issues where computations happen as follow-up work to the value coming out of the response. fixes #59352 PR Close #60656
diff --git a/packages/common/http/src/interceptor.ts b/packages/common/http/src/interceptor.ts
@@ -16,7 +16,7 @@ import {
   runInInjectionContext,
   &#629;Console as Console,
   &#629;formatRuntimeError as formatRuntimeError,
-  &#629;PendingTasksInternal as PendingTasks,
+  PendingTasks,
 } from '@angular/core';
 import {Observable} from 'rxjs';
 import {finalize} from 'rxjs/operators';
@@ -244,8 +244,8 @@ export function legacyInterceptorFnFactory(): HttpInterceptorFn {
     const pendingTasks = inject(PendingTasks);
     const contributeToStability = inject(REQUESTS_CONTRIBUTE_TO_STABILITY);
     if (contributeToStability) {
-      const taskId = pendingTasks.add();
-      return chain(req, handler).pipe(finalize(() => pendingTasks.remove(taskId)));
+      const removeTask = pendingTasks.add();
+      return chain(req, handler).pipe(finalize(removeTask));
     } else {
       return chain(req, handler);
     }
@@ -323,10 +323,10 @@ export class HttpInterceptorHandler extends HttpHandler {
     }
 
     if (this.contributeToStability) {
-      const taskId = this.pendingTasks.add();
+      const removeTask = this.pendingTasks.add();
       return this.chain(initialRequest, (downstreamRequest) =>
         this.backend.handle(downstreamRequest),
-      ).pipe(finalize(() => this.pendingTasks.remove(taskId)));
+      ).pipe(finalize(removeTask));
     } else {
       return this.chain(initialRequest, (downstreamRequest) =>
         this.backend.handle(downstreamRequest),
diff --git a/packages/common/http/test/provider_spec.ts b/packages/common/http/test/provider_spec.ts
@@ -26,6 +26,7 @@ import {
   ApplicationRef,
   createEnvironmentInjector,
   EnvironmentInjector,
+  ErrorHandler,
   inject,
   InjectionToken,
   PLATFORM_ID,
@@ -57,15 +58,35 @@ describe('without provideHttpClientTesting', () => {
     });
 
     let stable = false;
+    const appRef = TestBed.inject(ApplicationRef);
     TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
       stable = v;
     });
 
     expect(stable).toBe(true);
     TestBed.inject(HttpClient).get('/test', {responseType: 'text'}).subscribe();
     expect(stable).toBe(false);
-    await Promise.resolve();
-    expect(stable).toBe(true);
+    await expectAsync(appRef.whenStable()).toBeResolved();
+  });
+
+  it('can use an ErrorHandler that injects HttpClient', () => {
+    // This test ensures there is no circular dependency:
+    // possible loops are:
+    // INTERNAL_APPLICATION_ERROR_HANDLER -> ErrorHandler -> HttpClient -> PendingTasks -> ChangeDetectionScheduler -> INTERNAL_APPLICATION_ERROR_HANDLER
+    TestBed.configureTestingModule({
+      providers: [
+        provideHttpClient(withInterceptors([() => from(Promise.resolve(new HttpResponse()))])),
+        {
+          provide: ErrorHandler,
+          useClass: class extends ErrorHandler {
+            client = inject(HttpClient);
+            override handleError(error: any): void {}
+          },
+        },
+      ],
+    });
+
+    expect(() => TestBed.inject(ApplicationRef)).not.toThrow();
   });
 });
 
