My dev team encountered an issue of high memory usage linked to running for a long time a periodic GraphQL subscription using an ApolloClient that uses a RetryLink. The apparent source of the leak was traced to a private array of values inside RetryableOperation.
/**
* Tracking and management of operations that may be (or currently are) retried.
*/
class RetryableOperation<TValue = any> {
private retryCount: number = 0;
private values: any[] = [];
This array grows every time the request observable from the link receives a new value via next and is never reduced in length.
private onNext = (value: any) => {
this.values.push(value);
for (const observer of this.observers) {
if (!observer) continue;
observer.next!(value);
}
};
This array of values is then used to replay all values previously broadcast via next through the request observable to a new observer.
/**
* Register a new observer for this operation.
*
* If the operation has previously emitted other events, they will be
* immediately triggered for the observer.
*/
public subscribe(observer: Observer<TValue>) {
if (this.canceled) {
throw new Error(
`Subscribing to a retryable link that was canceled is not supported`
);
}
this.observers.push(observer);
// If we've already begun, catch this observer up.
for (const value of this.values) {
observer.next!(value);
}
if (this.complete) {
observer.complete!();
} else if (this.error) {
observer.error!(this.error);
}
}
What confuses me is that when I tracked the original commit adding this feature down to this apollo-link commit, the commit message states that the changes are for helping the RetryLink implementation pass some newly designed tests. However, upon pulling the repository at that commit, removing the lines of code for replaying previously broadcast values to new observers of the RetryLink request observable, and then running the test suite the associated pull request modified, the tests still pass.
diff --git a/packages/apollo-link-retry/src/retryLink.ts b/packages/apollo-link-retry/src/retryLink.ts
index 680a4772..f2c9c10d 100644
--- a/packages/apollo-link-retry/src/retryLink.ts
+++ b/packages/apollo-link-retry/src/retryLink.ts
@@ -70,7 +70,6 @@ export class RetryLink extends ApolloLink {
nextLink: NextLink,
): Observable<FetchResult> {
let retryCount = 0;
- const values = [];
let complete = false;
const observers = [];
let currentSubscription;
@@ -79,7 +78,6 @@ export class RetryLink extends ApolloLink {
const subscriber = {
next: data => {
retryCount = 0;
- values.push(data);
for (const observer of observers) {
observer.next(data);
}
@@ -109,9 +107,6 @@ export class RetryLink extends ApolloLink {
return new Observable(observer => {
observers.push(observer);
- for (const value of values) {
- observer.next(value);
- }
if (complete) {
observer.complete();
}
> apollo-link-retry@1.0.2 test
> jest
PASS src/__tests__/retryLink.ts
RetryLink
✓ fails for unreachable endpoints (14ms)
✓ returns data from the underlying link on a successful operation (1ms)
✓ returns data from the underlying link on a successful retry (1ms)
✓ calls unsubscribe on the appropriate downstream observable (2ms)
✓ supports multiple subscribers to the same request (3ms)
✓ retries independently for concurrent requests (5ms)
Test Suites: 1 passed, 1 total
Tests: 6 passed, 6 total
Snapshots: 0 total
Time: 0.762s, estimated 1s
Ran all test suites.
This past-value-replaying functionality within RetryableOperation is not publicly documented and appears to have been snuck into a past pull request for apollo-link without a publicly visible explanation at the time. If someone knows what the motivation is for adding such functionality, that would be greatly appreciated.