fix(http): Only contribute to ApplicationRef.isStable indicator in …

…the `HttpBackend`

This commit updates the approach to how the `http` package contributes
to application stability by moving the contribution point from an
interceptor wrapper to the backend.

`HttpClient` uses the `PendingTasks` service to contribute to
application stability. This was added in v16 to support SSR without
relying on an infinite `setTimeout` with ZoneJS like it did pre-v16.
Prior to version 16, this was also only done on the server and did not
affect clients or unit tests. (28c68f7)

=== Additional background information ===

Prior to #54949, `PendingTasks` contribute to `ApplicationRef.isStable` but did not
contribute to the stability of `ComponentFixture`. This divergence in
stability behavior was not intended. By aligning the two behaviors again, this
includes all pending tasks in the stability of fixtures. This is
likely to be a pretty large breaking change test with `HttpClient`.
Tests appear to quite often use `await fixture.whenStable` when there are
unfinished requests that have not been mocked or flushed.

This change prevents request in `HttpClient` from contributing to
stability through the `PendingTasks` in tests by not including the stability
contribution in the mock backend. In this scenario, requests need to be
flushed manually for them to resolve, which is problematic for existing
test suites which do not flush them before `await fixture.whenStable`.

BREAKING CHANGE: `HttpClient` interceptors are no longer contribute directly to
`ApplicationRef.isStable` without ZoneJS. `ApplicationRef.isStable` is
used for SSR to determine when to serialize the application. If there is
async work in interceptors that is not captured by ZoneJS and needs to
be included in the SSR serialization, these should be updated to keep
the zone unstable by running a timeout inside the zone and clearing it when
the async interceptor work is finished.

packages/common/http/src/fetch.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {inject, Injectable, NgZone} from '@angular/core';
+import {inject, Injectable, NgZone, ɵPendingTasks as PendingTasks} from '@angular/core';
 import {Observable, Observer} from 'rxjs';
+import {finalize} from 'rxjs/operators';
 
 import {HttpBackend} from './backend';
 import {HttpHeaders} from './headers';
@@ -56,15 +57,21 @@ export class FetchBackend implements HttpBackend {
   private readonly fetchImpl =
     inject(FetchFactory, {optional: true})?.fetch ?? fetch.bind(globalThis);
   private readonly ngZone = inject(NgZone);
+  private readonly pendingTasks = inject(PendingTasks);
 
   handle(request: HttpRequest<any>): Observable<HttpEvent<any>> {
-    return new Observable((observer) => {
+    const pendingTask = this.pendingTasks.add();
+    return new Observable<HttpEvent<any>>((observer) => {
       const aborter = new AbortController();
       this.doRequest(request, aborter.signal, observer).then(noop, (error) =>
         observer.error(new HttpErrorResponse({error})),
       );
       return () => aborter.abort();
-    });
+    }).pipe(
+      finalize(() => {
+        this.pendingTasks.remove(pendingTask);
+      }),
+    );
   }
 
   private async doRequest(

packages/common/http/src/interceptor.ts

@@ -16,7 +16,6 @@ import {
   runInInjectionContext,
   ɵConsole as Console,
   ɵformatRuntimeError as formatRuntimeError,
-  ɵPendingTasks as PendingTasks,
 } from '@angular/core';
 import {Observable} from 'rxjs';
 import {finalize} from 'rxjs/operators';
@@ -242,9 +241,7 @@ export function legacyInterceptorFnFactory(): HttpInterceptorFn {
       );
     }
 
-    const pendingTasks = inject(PendingTasks);
-    const taskId = pendingTasks.add();
-    return chain(req, handler).pipe(finalize(() => pendingTasks.remove(taskId)));
+    return chain(req, handler);
   };
 }
 
@@ -258,7 +255,6 @@ export function resetFetchBackendWarningFlag() {
 @Injectable()
 export class HttpInterceptorHandler extends HttpHandler {
   private chain: ChainedInterceptorFn<unknown> | null = null;
-  private readonly pendingTasks = inject(PendingTasks);
 
   constructor(
     private backend: HttpBackend,
@@ -316,9 +312,8 @@ export class HttpInterceptorHandler extends HttpHandler {
       );
     }
 
-    const taskId = this.pendingTasks.add();
     return this.chain(initialRequest, (downstreamRequest) =>
       this.backend.handle(downstreamRequest),
-    ).pipe(finalize(() => this.pendingTasks.remove(taskId)));
+    );
   }
 }

packages/common/http/src/jsonp.ts

@@ -13,8 +13,9 @@ import {
   inject,
   Injectable,
   runInInjectionContext,
+  ɵPendingTasks as PendingTasks,
 } from '@angular/core';
-import {Observable, Observer} from 'rxjs';
+import {finalize, Observable, Observer} from 'rxjs';
 
 import {HttpBackend, HttpHandler} from './backend';
 import {HttpHandlerFn} from './interceptor';
@@ -92,6 +93,7 @@ export class JsonpClientBackend implements HttpBackend {
    * A resolved promise that can be used to schedule microtasks in the event handlers.
    */
   private readonly resolvedPromise = Promise.resolve();
+  private readonly pendingTasks = inject(PendingTasks);
 
   constructor(
     private callbackMap: JsonpCallbackContext,
@@ -127,6 +129,7 @@ export class JsonpClientBackend implements HttpBackend {
     }
 
     // Everything else happens inside the Observable boundary.
+    const pendingTask = this.pendingTasks.add();
     return new Observable<HttpEvent<any>>((observer: Observer<HttpEvent<any>>) => {
       // The first step to make a request is to generate the callback name, and replace the
       // callback placeholder in the URL with the name. Care has to be taken here to ensure
@@ -251,7 +254,11 @@ export class JsonpClientBackend implements HttpBackend {
         // And finally, clean up the page.
         cleanup();
       };
-    });
+    }).pipe(
+      finalize(() => {
+        this.pendingTasks.remove(pendingTask);
+      }),
+    );
   }
 
   private removeListeners(script: HTMLScriptElement): void {

packages/common/http/src/xhr.ts

@@ -7,9 +7,14 @@
  */
 
 import {XhrFactory} from '@angular/common';
-import {Injectable, ɵRuntimeError as RuntimeError} from '@angular/core';
+import {
+  Injectable,
+  ɵRuntimeError as RuntimeError,
+  ɵPendingTasks as PendingTasks,
+  inject,
+} from '@angular/core';
 import {from, Observable, Observer, of} from 'rxjs';
-import {switchMap} from 'rxjs/operators';
+import {finalize, switchMap} from 'rxjs/operators';
 
 import {HttpBackend} from './backend';
 import {RuntimeErrorCode} from './errors';
@@ -52,6 +57,7 @@ function getResponseUrl(xhr: any): string | null {
  */
 @Injectable()
 export class HttpXhrBackend implements HttpBackend {
+  private readonly pendingTasks = inject(PendingTasks);
   constructor(private xhrFactory: XhrFactory) {}
 
   /**
@@ -80,6 +86,7 @@ export class HttpXhrBackend implements HttpBackend {
 
     return source.pipe(
       switchMap(() => {
+        const pendingTask = this.pendingTasks.add();
         // Everything happens on Observable subscription.
         return new Observable((observer: Observer<HttpEvent<any>>) => {
           // Start by setting up the XHR object with request method, URL, and withCredentials
@@ -347,7 +354,11 @@ export class HttpXhrBackend implements HttpBackend {
               xhr.abort();
             }
           };
-        });
+        }).pipe(
+          finalize(() => {
+            this.pendingTasks.remove(pendingTask);
+          }),
+        );
       }),
     );
   }

packages/common/http/test/fetch_spec.ts

@@ -8,8 +8,8 @@
 
 import {HttpEvent, HttpEventType, HttpRequest, HttpResponse} from '@angular/common/http';
 import {TestBed} from '@angular/core/testing';
-import {Observable, of, Subject} from 'rxjs';
-import {catchError, retry, scan, skip, take, toArray} from 'rxjs/operators';
+import {firstValueFrom, Observable, of, Subject} from 'rxjs';
+import {catchError, filter, retry, scan, skip, take, toArray} from 'rxjs/operators';
 
 import {
   HttpDownloadProgressEvent,
@@ -19,6 +19,7 @@ import {
   HttpStatusCode,
 } from '../public_api';
 import {FetchBackend, FetchFactory} from '../src/fetch';
+import {ApplicationRef} from '@angular/core';
 
 function trackEvents(obs: Observable<any>): Promise<any[]> {
   return obs
@@ -68,6 +69,72 @@ describe('FetchBackend', async () => {
     backend = TestBed.inject(FetchBackend);
   });
 
+  describe('stability', () => {
+    const post = TEST_POST.clone({
+      setHeaders: {
+        'Test': 'Test header',
+      },
+    });
+    function whenStable() {
+      return firstValueFrom(TestBed.inject(ApplicationRef).isStable.pipe(filter((s) => !!s)));
+    }
+
+    it('should contribute to stability until backend request completes', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(TEST_POST).subscribe();
+      expect(stable).toBe(false);
+      fetchMock.mockFlush(HttpStatusCode.Ok, 'OK', 'some response');
+      await expectAsync(whenStable()).toBeResolved();
+    });
+
+    it('should contribute to stability on progress events', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(TEST_POST).subscribe();
+      expect(stable).toBe(false);
+      fetchMock.mockProgressEvent(4);
+      await new Promise<void>((resolve) => setTimeout(resolve));
+      expect(stable).toBe(false);
+      fetchMock.mockFlush(HttpStatusCode.Ok, 'OK', 'some response');
+      await expectAsync(whenStable()).toBeResolved();
+    });
+
+    it('should contribute to stability until backend request errors', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(TEST_POST).subscribe({error: () => {}});
+      expect(stable).toBe(false);
+      fetchMock.mockErrorEvent(HttpStatusCode.Ok);
+      await expectAsync(whenStable()).toBeResolved();
+    });
+
+    it('should contribute to stability until backend request aborts', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(TEST_POST).subscribe({error: () => {}});
+      expect(stable).toBe(false);
+      fetchMock.mockAbortEvent();
+      await expectAsync(whenStable()).toBeResolved();
+    });
+  });
+
   it('emits status immediately', () => {
     let event!: HttpEvent<any>;
     // subscribe is sync
@@ -475,6 +542,7 @@ export class MockFetchFactory extends FetchFactory {
       Object.defineProperty(response, 'url', {value: this.response.url});
     }
     this.resolve(response);
+    this.promise;
   }
 
   mockProgressEvent(loaded: number): void {

packages/common/http/test/jsonp_spec.ts

@@ -16,9 +16,12 @@ import {
 } from '@angular/common/http/src/jsonp';
 import {HttpRequest} from '@angular/common/http/src/request';
 import {HttpErrorResponse, HttpEventType} from '@angular/common/http/src/response';
-import {toArray} from 'rxjs/operators';
+import {filter, toArray} from 'rxjs/operators';
 
 import {MockDocument} from './jsonp_mock';
+import {TestBed} from '@angular/core/testing';
+import {firstValueFrom} from 'rxjs';
+import {ApplicationRef} from '@angular/core';
 
 describe('JsonpClientBackend', () => {
   const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
@@ -36,7 +39,40 @@ describe('JsonpClientBackend', () => {
   beforeEach(() => {
     home = {};
     document = new MockDocument();
-    backend = new JsonpClientBackend(home, document);
+    backend = TestBed.runInInjectionContext(() => new JsonpClientBackend(home, document));
+  });
+
+  describe('stability', () => {
+    function whenStable() {
+      return firstValueFrom(TestBed.inject(ApplicationRef).isStable.pipe(filter((s) => !!s)));
+    }
+
+    it('should contribute to stability until backend request completes', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(SAMPLE_REQ).subscribe();
+      expect(stable).toBe(false);
+      runOnlyCallback(home, {data: 'This is a test'});
+      document.mockLoad();
+      await expectAsync(whenStable()).toBeResolved();
+    });
+
+    it('should contribute to stability until backend request errors', async () => {
+      let stable = false;
+      TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+        stable = v;
+      });
+
+      expect(stable).toBe(true);
+      backend.handle(SAMPLE_REQ).subscribe({error: () => {}});
+      expect(stable).toBe(false);
+      document.mockLoad();
+      await expectAsync(whenStable()).toBeResolved();
+    });
   });
 
   it('handles a basic request', (done) => {

packages/common/http/test/provider_spec.ts

@@ -26,6 +26,7 @@ import {
   provideHttpClientTesting,
 } from '@angular/common/http/testing';
 import {
+  ApplicationRef,
   createEnvironmentInjector,
   EnvironmentInjector,
   inject,
@@ -34,7 +35,7 @@ import {
   Provider,
 } from '@angular/core';
 import {TestBed} from '@angular/core/testing';
-import {EMPTY, Observable} from 'rxjs';
+import {EMPTY, Observable, from} from 'rxjs';
 
 import {HttpInterceptorFn, resetFetchBackendWarningFlag} from '../src/interceptor';
 import {
@@ -76,6 +77,23 @@ describe('provideHttpClient', () => {
     TestBed.inject(HttpTestingController).expectOne('/test').flush('');
   });
 
+  it('should not contribute to stability', () => {
+    TestBed.configureTestingModule({
+      providers: [provideHttpClient(), provideHttpClientTesting()],
+    });
+
+    let stable = false;
+    TestBed.inject(ApplicationRef).isStable.subscribe((v) => {
+      stable = v;
+    });
+
+    expect(stable).toBe(true);
+    TestBed.inject(HttpClient).get('/test', {responseType: 'text'}).subscribe();
+    expect(stable).toBe(true);
+    TestBed.inject(HttpTestingController).expectOne('/test').flush('');
+    expect(stable).toBe(true);
+  });
+
   it('should not use legacy interceptors by default', () => {
     TestBed.configureTestingModule({
       providers: [