8000 fix(common): abort HTTP requests once application is destroyed · angular/angular@7cf3bcc · GitHub
[go: up one dir, main page]
Skip to content < 8000 link crossorigin="anonymous" media="all" rel="stylesheet" href="https://github.githubassets.com/assets/primer-react.47f1598729334a521d2a.module.css" />

Commit 7cf3bcc

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 7cf3bcc

File tree

4 files changed

+73
-5
lines changed

4 files changed

+73
-5
lines changed

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

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,16 @@
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+
ɵformatRuntimeError as formatRuntimeError,
16+
} from '@angular/core';
17+
import {type Observable, of, Subject} from 'rxjs';
18+
import {concatMap, filter, map, takeUntil} from 'rxjs/operators';
1219

1320
import {HttpHandler} from './backend';
1421
import {HttpContext} from './context';
@@ -113,7 +120,24 @@ function addBody<T>(
113120
*/
114121
@Injectable()
115122
export class HttpClient {
116-
constructor(private handler: HttpHandler) {}
123+
private readonly destroyed$ = new Subject<void>();
124+
125+
constructor(private handler: HttpHandler) {
126+
// Attempt to retrieve a `DestroyRef` optionally.
127+
// For backwards compatibility reasons, this cannot be required.
128+
if (isInInjectionContext()) {
129+
inject(DestroyRef, {optional: true})?.onDestroy(() => this.destroyed$.next());
130+
} else if (typeof ngDevMode !== 'undefined' && ngDevMode) {
131+
console.warn(
132+
formatRuntimeError(
133+
RuntimeErrorCode.HTTP_CLIENT_MISSING_INJECTION_CONTEXT,
134+
'Angular detected that HttpClient is being instantiated outside of an injection context. ' +
135+
'As a result, automatic cleanup via DestroyRef will not be performed. ' +
136+
'This may lead to memory leaks.',
137+
),
138+
);
139+
}
140+
}
117141

118142
/**
119143
* Sends an `HttpRequest` and returns a stream of `HttpEvent`s.
@@ -681,6 +705,7 @@ export class HttpClient {
681705
// subscription (this also makes retries re-run the handler, including interceptors).
682706
const events$: Observable<HttpEvent<any>> = of(req).pipe(
683707
concatMap((req: HttpRequest<any>) => this.handler.handle(req)),
708+
takeUntil(this.destroyed$),
684709
);
685710

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@ export const enum RuntimeErrorCode {
2525
JSONP_WRONG_RESPONSE_TYPE = 2811,
2626
JSONP_HEADERS_NOT_SUPPORTED = 2812,
2727
KEEPALIVE_NOT_SUPPORTED_WITH_XHR = 2813,
28+
HTTP_CLIENT_MISSING_INJECTION_CONTEXT = 2814,
2829
}

‎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