8000 fix(common): abort HTTP requests once application is destroyed · angular/angular@e9a0c27 · GitHub
[go: up one dir, main page]
Skip to content

Commit e9a0c27

Browse files
arturovtarturovt
committed
fix(common): abort HTTP requests once application is destroyed
In this commit, we abort any pending HTTP requests when the application is destroyed to prevent memory leaks and the execution of operations that should not occur once resources are released. For example, if an HTTP response is returned after the application is destroyed and someone calls `runInInjectionContext`, it would throw an error indicating that the injector has already been destroyed. This ensures a graceful cleanup, ensuring that no requests proceed once the root injector is destroyed.
1 parent 935ce0e commit e9a0c27

File tree

3 files changed

+62
-5
lines changed

3 files changed

+62
-5
lines changed

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,15 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {Injectable, ɵRuntimeError as RuntimeError} from '@angular/core';
10-
import {Observable, of} from 'rxjs';
11-
import {concatMap, filter, map} from 'rxjs/operators';
9+
import {
10+
DestroyRef,
11+
inject,
12+
Injectable,
13+
ɵRuntimeError as RuntimeError,
14+
ɵisInInjectionContext as isInInjectionContext,
15+
} from '@angular/core';
16+
import {type Observable, of, Subject} from 'rxjs';
17+
import {concatMap, filter, map, takeUntil} from 'rxjs/operators';
1218

1319
import {HttpHandler} from './backend';
1420
import {HttpContext} from './context';
@@ -113,7 +119,15 @@ function addBody<T>(
113119
*/
114120
@Injectable()
115121
export class HttpClient {
116-
constructor(private handler: HttpHandler) {}
122+
private readonly destroyed$ = new Subject<void>();
123+
124+
constructor(private handler: HttpHandler) {
125+
// Attempt to retrieve a `DestroyRef` optionally.
126+
// For backwards compatibility reasons, this cannot be required.
127+
if (isInInjectionContext()) {
128+
inject(DestroyRef, {optional: true})?.onDestroy(() => this.destroyed$.next());
129+
}
130+
}
117131

118132
/**
119133
* Sends an `HttpRequest` and returns a stream of `HttpEvent`s.
@@ -681,6 +695,7 @@ export class HttpClient {
681695
// subscription (this also makes retries re-run the handler, including interceptors).
682696
const events$: Observable<HttpEvent<any>> = of(req).pipe(
683697
concatMap((req: HttpRequest<any>) => this.handler.handle(req)),
698+
takeUntil(this.destroyed$),
684699
);
685700

686701
// If coming via the API signature which accepts a previously constructed HttpRequest,

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,44 @@ describe('FetchBackend', async () => {
305305
fetchMock.mockFlush(HttpStatusCode.Ok, 'OK');
306306
});
307307

308+
describe('root injector is destroyed', () => {
309+
it('aborts a request once root injector is destroyed', async () => {
310+
const assertion = {
311+
abortHappened: false,
312+
error: <HttpErrorResponse | null>null,
313+
};
314+
315+
const request = new HttpRequest('GET', '/test');
316+
backend.handle(request).subscribe({
317+
error: (error: HttpErrorResponse) => {
318+
assertion.abortHappened = true;
319+
assertion.error = error;
320+
},
321+
});
322+
323+
expect(assertion).toEqual({abortHappened: false, error: null});
324+
325+
// We still need to manually reject the promise because we are in a unit test environment.
326+
// However, the unit test ensures that `abort()` is called on the abort controller
327+
// when the root injector is destroyed.
328+
// The `mockAbortEvent` is unrelated to abort controllers;
329+
// it is only responsible for rejecting a promise.
330+
fetchMock.mockAbortEvent();
331+
332+
// `resetTestingModule` triggers the destruction of the root injector.
333+
// We cannot inject the `ApplicationRef` and invoke `destroy()` because
334+
// it would attempt to call `resetTestingModule()` and throw an error
335+
// indicating that the injector has already been destroyed.
336+
TestBed.resetTestingModule();
337+
338+
// Wait until a promise is rejected.
339+
await Promise.resolve();
340+
341+
expect(assertion.abortHappened).toEqual(true);
342+
expect(assertion.error!.error).toBeInstanceOf(DOMException);
343+
});
344+
});
345+
308346
describe('progress events', () => {
309347
it('are emitted for download progress', (done) => {
310348
backend

‎packages/core/src/di/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
*/
1414

1515
export * from './metadata';
16-
export {assertInInjectionContext, runInInjectionContext} from './contextual';
16+
export {
17+
isInInjectionContext as ɵisInInjectionContext,
18+
assertInInjectionContext,
19+
runInInjectionContext,
20+
} from './contextual';
1721
export {
1822
ɵɵdefineInjectable,
1923
defineInjectable,

0 commit comments

Comments
 (0)
0