merged timeout managers into one

Author: toptobes

src/api/data-api-http-client.ts

@@ -17,8 +17,6 @@ import { DataAPIResponseError, DataAPITimeout, mkRespErrorFromResponse, ObjectId
 import { logger } from '@/src/logger';
 import {
   MkTimeoutError,
-  MultiCallTimeoutManager,
-  SingleCallTimeoutManager,
   TimeoutManager,
   TimeoutOptions,
 } from '@/src/api/timeout-managers';
@@ -40,12 +38,12 @@ export class DataApiHttpClient extends HttpClient {
   public collection?: string;
   public namespace?: string;
 
-  public multiCallTimeoutManager(timeoutMs: number | undefined) {
-    return mkTimeoutManager(MultiCallTimeoutManager, timeoutMs);
+  public timeoutManager(timeoutMs: number | undefined) {
+    return mkTimeoutManager(timeoutMs);
   }
 
   public async executeCommand(command: Record<string, any>, options: ExecuteCommandOptions | undefined) {
-    const timeoutManager = options?.timeoutManager ?? mkTimeoutManager(SingleCallTimeoutManager, options?.maxTimeMS);
+    const timeoutManager = options?.timeoutManager ?? mkTimeoutManager(options?.maxTimeMS);
 
     const response = await this._requestDataApi({
       url: this.baseUrl,
@@ -104,9 +102,9 @@ export class DataApiHttpClient extends HttpClient {
   }
 }
 
-const mkTimeoutManager = (constructor: new (maxMs: number, mkTimeoutError: MkTimeoutError) => TimeoutManager, maxMs: number | undefined) => {
+const mkTimeoutManager = (maxMs: number | undefined) => {
   const timeout = maxMs ?? DEFAULT_TIMEOUT;
-  return new constructor(timeout, mkTimeoutErrorMaker(timeout));
+  return new TimeoutManager(timeout, mkTimeoutErrorMaker(timeout));
 }
 
 const mkTimeoutErrorMaker = (timeout: number): MkTimeoutError => {

src/api/devops-api-http-client.ts

@@ -19,10 +19,7 @@ import { HTTP1AuthHeaderFactories, HTTP1Strategy } from '@/src/api/http1';
 import { DevopsApiResponseError, DevopsApiTimeout, DevopsUnexpectedStateError } from '@/src/devops/errors';
 import { AdminBlockingOptions, PollBlockingOptions } from '@/src/devops/types';
 import {
-  MkTimeoutError,
-  MultiCallTimeoutManager,
-  SingleCallTimeoutManager,
-  TimeoutManager,
+  MkTimeoutError, TimeoutManager,
   TimeoutOptions,
 } from '@/src/api/timeout-managers';
 import { HttpMethods } from '@/src/api/constants';
@@ -50,7 +47,7 @@ export class DevopsApiHttpClient extends HttpClient {
 
   public async request(info: DevopsApiRequestInfo, options: TimeoutOptions | undefined): Promise<AxiosResponse> {
     try {
-      const timeoutManager = options?.timeoutManager ?? mkTimeoutManager(SingleCallTimeoutManager, options?.maxTimeMS);
+      const timeoutManager = options?.timeoutManager ?? mkTimeoutManager(options?.maxTimeMS);
       const url = this.baseUrl + info.path;
 
       return await this._request({
@@ -69,7 +66,7 @@ export class DevopsApiHttpClient extends HttpClient {
   }
 
   public async requestLongRunning(req: DevopsApiRequestInfo, info: LongRunningRequestInfo): Promise<AxiosResponse> {
-    const timeoutManager = mkTimeoutManager(MultiCallTimeoutManager, info.options?.maxTimeMS);
+    const timeoutManager = mkTimeoutManager(info.options?.maxTimeMS);
     const resp = await this.request(req, { timeoutManager });
 
     const id = (typeof info.id === 'function')
@@ -107,9 +104,9 @@ export class DevopsApiHttpClient extends HttpClient {
   }
 }
 
-const mkTimeoutManager = (constructor: new (maxMs: number, mkTimeoutError: MkTimeoutError) => TimeoutManager, maxMs: number | undefined) => {
+const mkTimeoutManager = (maxMs: number | undefined) => {
   const timeout = maxMs ?? 0;
-  return new constructor(timeout, mkTimeoutErrorMaker(timeout));
+  return new TimeoutManager(timeout, mkTimeoutErrorMaker(timeout));
 }
 
 const mkTimeoutErrorMaker = (timeout: number): MkTimeoutError => {

src/api/timeout-managers.ts

@@ -43,50 +43,14 @@ export type TimeoutOptions = {
  */
 export type MkTimeoutError = (ctx: InternalHTTPRequestInfo) => Error;
 
-/**
- * Represents a timeout manager, which is responsible for tracking the remaining time before a timeout occurs.
- *
- * See {@link SingleCallTimeoutManager} and {@link MultiCallTimeoutManager} for concrete implementations.
- *
- * @internal
- */
-export interface TimeoutManager {
-  /**
-   * The remaining time before a timeout occurs, in milliseconds. May return `Infinity`, which
-   * the underlying http request strategies should interpret as no timeout.
-   *
-   * However, the implementing TimeoutManager classes should accept `0` to also indicate no timeout,
-   * just translating it to `Infinity` for the purposes of the `msRemaining` getter, so it's easier to
-   * reason about.
-   */
-  msRemaining: number,
-  /**
-   * A function that creates a timeout error for the given request context ({@link InternalHTTPRequestInfo}).
-   */
-  mkTimeoutError: MkTimeoutError,
-}
-
-/**
- * A basic timeout manager that only holds a fixed timeout value, intended for, of course, single-request operations.
- *
- * @internal
- */
-export class SingleCallTimeoutManager implements TimeoutManager {
-  public readonly msRemaining: number;
-
-  constructor(maxMs: number, readonly mkTimeoutError: MkTimeoutError) {
-    this.msRemaining = maxMs || Infinity;
-  }
-}
-
 /**
  * A more complex timeout manager that tracks the remaining time for multiple calls, starting from the first call.
  * This is useful for scenarios where multiple calls are made in sequence, and the timeout should be shared among them,
  * e.g. {@link Collection.insertMany}.
  *
  * @internal
  */
-export class MultiCallTimeoutManager implements TimeoutManager {
+export class TimeoutManager {
   private _deadline!: number;
   private _started: boolean;
 
@@ -98,7 +62,9 @@ export class MultiCallTimeoutManager implements TimeoutManager {
   get msRemaining() {
     if (!this._started) {
       this._started = true;
-      this._deadline = Date.now() + this._deadline;
+      const maxMs = this._deadline;
+      this._deadline = Date.now() + maxMs;
+      return maxMs
     }
     return this._deadline - Date.now();
   }

src/data-api/collection.ts

@@ -297,7 +297,7 @@ export class Collection<Schema extends SomeDoc = SomeDoc> {
       }
     }
 
-    const timeoutManager = this._httpClient.multiCallTimeoutManager(options?.maxTimeMS);
+    const timeoutManager = this._httpClient.timeoutManager(options?.maxTimeMS);
 
     const insertedIds = (options?.ordered)
       ? await insertManyOrdered<Schema>(this._httpClient, documents, chunkSize, timeoutManager)
@@ -449,7 +449,7 @@ export class Collection<Schema extends SomeDoc = SomeDoc> {
       },
     };
 
-    const timeoutManager = this._httpClient.multiCallTimeoutManager(options?.maxTimeMS);
+    const timeoutManager = this._httpClient.timeoutManager(options?.maxTimeMS);
 
     const commonResult = {
       modifiedCount: 0,
@@ -660,7 +660,7 @@ export class Collection<Schema extends SomeDoc = SomeDoc> {
       deleteMany: { filter },
     };
 
-    const timeoutManager = this._httpClient.multiCallTimeoutManager(options?.maxTimeMS);
+    const timeoutManager = this._httpClient.timeoutManager(options?.maxTimeMS);
 
     let resp;
     let numDeleted = 0;
@@ -1398,7 +1398,7 @@ export class Collection<Schema extends SomeDoc = SomeDoc> {
    * @throws BulkWriteError - If the operation fails
    */
   public async bulkWrite(operations: AnyBulkWriteOperation<Schema>[], options?: BulkWriteOptions): Promise<BulkWriteResult> {
-    const timeoutManager = this._httpClient.multiCallTimeoutManager(options?.maxTimeMS)
+    const timeoutManager = this._httpClient.timeoutManager(options?.maxTimeMS)
 
     return (options?.ordered)
       ? await bulkWriteOrdered(this._httpClient, operations, timeoutManager)

src/data-api/db.ts

@@ -311,7 +311,7 @@ export class Db implements Disposable {
       },
     };
 
-    const timeoutManager = this._httpClient.multiCallTimeoutManager(options?.maxTimeMS);
+    const timeoutManager = this._httpClient.timeoutManager(options?.maxTimeMS);
     const namespace = options?.namespace ?? this.namespace;
 
     if (options?.checkExists !== false) {

tests/integration/api/devops-http-client.test.ts

@@ -26,14 +26,12 @@ describe('integration.api.devops-http-client', () => {
     httpClient = db.admin()['_httpClient'];
   });
 
-  describe('request tests', () => {
-    it('should timeout properly', async () => {
-      await assert.rejects(async () => {
-        await httpClient.request({
-          method: HttpMethods.Get,
-          path: '/databases',
-        }, { maxTimeMS: 1 });
-      }, DevopsApiTimeout);
-    });
+  it('should timeout properly', async () => {
+    await assert.rejects(async () => {
+      await httpClient.request({
+        method: HttpMethods.Get,
+        path: '/databases',
+      }, { maxTimeMS: 1 });
+    }, DevopsApiTimeout);
   });
 });

tests/integration/devops/lifecycle.test.ts

@@ -17,8 +17,8 @@ import { assertTestsEnabled, initTestObjects } from '@/tests/fixtures';
 import { DataApiClient } from '@/src/client';
 import assert from 'assert';
 import { DevopsApiResponseError } from '@/src/devops';
-import { SingleCallTimeoutManager } from '@/src/api/timeout-managers';
 import { DEFAULT_NAMESPACE, DEFAULT_TIMEOUT } from '@/src/api';
+import { TimeoutManager } from '@/src/api/timeout-managers';
 
 describe('integration.devops.lifecycle', async () => {
   let client: DataApiClient;
@@ -28,23 +28,11 @@ describe('integration.devops.lifecycle', async () => {
 
     [client] = await initTestObjects(this);
 
-    // const dbs = await client.admin().listDatabases();
-    //
-    // if (dbs.some(db => db.info.name === 'astra-test-db')) {
-    //   throw new Error('Database \'astra-test-db\' already exists, drop it to proceed w/ lifecycle test');
-    // }
-  });
-
-  it('sdfsadfsda', async () => {
-    const admin = client.admin();
+    const dbs = await client.admin().listDatabases();
 
-    const resp = await admin.createDatabase({
-      name: 'astra-test-db',
-      cloudProvider: 'GCP',
-      region: 'us-east1',
-    });
-
-    console.log(resp.id);
+    if (dbs.some(db => db.info.name === 'astra-test-db')) {
+      throw new Error('Database \'astra-test-db\' already exists, drop it to proceed w/ lifecycle test');
+    }
   });
 
   it('[admin] works', async () => {
@@ -103,7 +91,7 @@ describe('integration.devops.lifecycle', async () => {
       }
 
       {
-        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['INITIALIZING', 'PENDING'], {}, 10000, new SingleCallTimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
+        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['INITIALIZING', 'PENDING'], {}, 10000, new TimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
       }
 
       for (const [dbAdmin, db, dbType] of [[syncDbAdmin, syncDb, 'sync'], [asyncDbAdmin, asyncDb, 'async']] as const) {
@@ -146,7 +134,7 @@ describe('integration.devops.lifecycle', async () => {
 
       {
         await syncDbAdmin.createNamespace('other_namespace');
-        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['MAINTENANCE'], {}, 1000, new SingleCallTimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
+        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['MAINTENANCE'], {}, 1000, new TimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
       }
 
       for (const [dbAdmin, db, dbType] of [[syncDbAdmin, syncDb, 'sync'], [asyncDbAdmin, asyncDb, 'async']] as const) {
@@ -165,7 +153,7 @@ describe('integration.devops.lifecycle', async () => {
 
       {
         await syncDbAdmin.dropNamespace('other_namespace', { blocking: true });
-        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['MAINTENANCE'], {}, 1000, new SingleCallTimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
+        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'ACTIVE', ['MAINTENANCE'], {}, 1000, new TimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
       }
 
       for (const [dbAdmin, db, dbType] of [[syncDbAdmin, syncDb, 'sync'], [asyncDbAdmin, asyncDb, 'async']] as const) {
@@ -186,7 +174,7 @@ describe('integration.devops.lifecycle', async () => {
 
       {
         await admin.dropDatabase(syncDb);
-        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'TERMINATED', ['TERMINATING'], {}, 10000, new SingleCallTimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
+        await asyncDbAdmin['_httpClient']['_awaitStatus'](asyncDb.id, 'TERMINATED', ['TERMINATING'], {}, 10000, new TimeoutManager(DEFAULT_TIMEOUT, () => new Error('Timeout')));
       }
 
       for (const [dbAdmin, dbType] of [[syncDbAdmin, 'sync'], [asyncDbAdmin, 'async']] as const) {

tests/unit/api/timeout-manager.test.ts

@@ -0,0 +1,30 @@
+// Copyright DataStax, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// noinspection DuplicatedCode
+
+import assert from 'assert';
+import { TimeoutManager } from '@/src/api/timeout-managers';
+
+describe('unit.api.timeout-manager', () => {
+  it('works', async () => {
+    const timeoutManager = new TimeoutManager(1000, () => new Error('timeout'));
+    assert.strictEqual(timeoutManager.msRemaining, 1000);
+    await new Promise((resolve) => setTimeout(resolve, 500));
+    assert.ok(timeoutManager.msRemaining < 500);
+    assert.ok(timeoutManager.msRemaining > 490);
+    await new Promise((resolve) => setTimeout(resolve, 500));
+    assert.ok(timeoutManager.msRemaining < 0);
+    assert.ok(timeoutManager.msRemaining > -20);
+  });
+});