Skip to content

Add configurable per-request timeouts to FamisClient (no-timeout hang on stalled requests) #107

Description

@alphahlee

SDK ticket: add configurable per-request timeouts to FamisClient (no-timeout hang)

Target repo: apptreesoftware/facility360_nodesdk (facility360 npm package)
Filed from: facility360_r7 connector · discovered in the 2026-07-02 full all-apps cache run
Type: Bug / hardening · Priority: High (affects the nightly cache and the live mobile user path)
Reference implementation: proposed patch in this repo at docs/operations/famis-sdk-request-timeout.patch (drafted against famis_client.ts + model/request_context.ts, builds clean, 68/68 SDK tests pass). master was intentionally not modified — the patch is a reference for the SDK team to place/adjust as they see fit.


Summary

FamisClient creates its axios instance with no timeout (axios default 0 = wait forever) and validateStatus: () => true. As a result, any single HTTP request that connects but is slow to respond — a request queued server-side at a saturated FAMIS tenant backend, or a call to an unhealthy endpoint — hangs indefinitely, holding the caller's slot until something external (e.g. Cloud Run's request timeout) kills it. We need a per-request timeout (bounding one HTTP round-trip, not a whole paginated operation), configurable, with a tighter default for login, and a per-call override for the rare legitimately-long single request.

Evidence (from the connector's full all-apps run, 2026-07-02)

Two failure modes, same root cause:

  1. crew_user_association on Crews-disabled tenants — the endpoint returns "Installation settings do not allow access of the Crews module." in <5s when the backend is idle (100 runs), but under concurrent load the same request sat queued 30–73 min before returning (86 runs), because the tenant's backend was busy with that tenant's own heavy queries. Bimodal timing on an identical error ⇒ the wait is transport/backend queueing, not the endpoint.

  2. Authorization Failed on kc-nafm.360engineer — a tenant with bad/unhealthy auth. All 10 loaders hung ~73 min each on the login call (withLoginCredential → login HTTP), then fast-failed on retry. The login step has the same no-timeout exposure.

Illustrative timeline (pepsi.360engineer — crew request concurrent with the tenant's heavy loaders):

crew_user_association   FAIL  05:04:03 → 06:04:10  (60m, then instant module error)
user_region_association ok    05:04:10 → 06:35:35  (91m,  ~2M rows)
user_property_assoc'ns  ok    05:04:15 → 06:57:43  (113m, ~2.7M rows)

Full analysis: famis-full-run-results-2026-07-02.md.

Why no timeout today, and why it's safe to add one

A naive total/operation timeout would break legitimate long fetches — which is almost certainly why none was set. But that conflates two durations:

  • Operation duration — a full paginated fetch (legitimately minutes–hours).
  • Request duration — one HTTP round-trip.

The SDK paginates at $top = 1000 (getAllBatch / getAllPaged, concurrent pages via Bottleneck maxConcurrent: 4). So a multi-hour fetch is many individually-short requests, not one long one. A per-request timeout therefore leaves long fetches untouched while catching a single stalled request. This is the crux of the fix.

Scope — this is SDK-wide, not cache-only

withAccessToken(...) (used for interactive mobile-user requests — list generation, submissions, live asset lookups) builds the same FamisClient/axios as the service-account path, so the no-timeout exposure applies to live user requests too: a stalled FAMIS response hangs the user's request (endless spinner / stacked retries), holds a connector slot, and at scale can exhaust instances for that tenant. The cache nightly run is itself a load event on the same backends, so the two contend. Only an SDK-level timeout protects the interactive path (there's no fan-out to gate there).

Recommended fix (single, cohesive change — bundle all of the below)

All of these share one surface (the axios request config) and should ship together. The reference patch implements items 1–5; 6–7 are recommended to include in the same effort.

  1. Per-request default timeout on the client axios instance. axios.create({ …, timeout: requestTimeoutMs }) in the FamisClient constructor. axios timeout is per-request and, on firing, destroys the socket (rejects ECONNABORTED), freeing the slot — which a caller-side Promise.race cannot do. Suggested default DEFAULT_REQUEST_TIMEOUT_MS = 120_000 (a 1,000-row page returns in seconds; 120s is huge headroom yet catches the 30–73 min stalls).
    • Where: famis_client.ts constructor this.http = axios.create({...}) (currently ~line 343).
  2. Tighter login timeout. The static login() axios instance has no timeout; a hung login is never legitimate. Suggested DEFAULT_LOGIN_TIMEOUT_MS = 30_000.
    • Where: famis_client.ts static async login() axios.create({ baseURL }) (currently ~line 279).
  3. Factory options. withLoginCredential({ …, requestTimeoutMs?, loginTimeoutMs? }) and withAccessToken({ …, requestTimeoutMs? }), threaded to the constructor / login().
  4. Per-call override. QueryContext.setTimeout(ms) (+ timeoutMs field), applied via a small timeoutConfig(context) helper passed to this.http.get(url, config) in the fetch paths — for the rare non-paginated endpoint whose single response legitimately runs long. Overrides the client default; unset ⇒ default applies.
    • Where: model/request_context.ts; wire into getAllBatch, getAllPaged (and ideally the other this.http.get(...) fetch helpers) in famis_client.ts.
  5. Retry interaction. With autoRetry, axios-retry (retries: 2) will see a timeout as retryable — cap/decide retry-on-timeout so a stall doesn't multiply. Suggest at most one retry on timeout.
  6. AbortSignal support (recommended, same effort). Accept an optional AbortSignal per call so callers can cancel in-flight requests (e.g. a mobile user navigates away) rather than leaving them to run to the timeout.
  7. (Optional v2) idle/inactivity timeout. If a hard per-request timeout proves too blunt for a legitimately slow-but-progressing single response, add a "no bytes received for N seconds" timeout (agent/stream level) so only truly stalled streams trip. Ship the simple per-request timeout first; only add this if needed.

Defaults & rollout

  • Ship on by default but generous (120s request / 30s login), fully configurable.
  • Instrument first: the SDK already exposes onComplete(info) with the request URL — capture per-request duration to confirm the single-request latency distribution and tune the default from data (we only have per-loader timings, which are many pages combined).
  • Backwards compatible: existing callers get the defaults automatically; no API break.

Connector impact once released

No connector code change is required to benefit — bumping the facility360 dependency applies the default timeout to every createSuperUserService (service-account) and withAccessToken (user) client automatically. The connector may optionally lower the default or use QueryContext.setTimeout(...) for specific heavy calls. (Connector-side complements already in place / considered: crew module pre-check shipped; an optional cacheApp auth-gate — neither replaces this SDK fix for the interactive path.)

Verification of the reference patch

  • yarn build (tsc): clean.
  • yarn test: 68/68 pass, incl. new QueryContext.setTimeout coverage (override carried, not serialized into the OData URL; defaults to undefined).
  • master left untouched; changes captured as a patch only.

Proposed diff

See docs/operations/famis-sdk-request-timeout.patch — apply in the SDK repo with git apply from the repo root. Touches famis_client.ts, model/request_context.ts, tests/query_context.test.ts (+71 / −3).


Inlined reference patch

Drafted against master (builds clean, 68/68 tests); apply with git apply from the repo root. Provided as a starting point — place/adjust as the team sees fit.

diff --git a/famis_client.ts b/famis_client.ts
index 0c79050..2e2c1ce 100644
--- a/famis_client.ts
+++ b/famis_client.ts
@@ -2,6 +2,7 @@ import {
   default as Axios,
   AxiosError,
   AxiosInstance,
+  AxiosRequestConfig,
   AxiosResponse,
   Method,
   default as axios,
@@ -187,6 +188,22 @@ export const DefaultPropertyExpand = [
 ];
 export const DefaultSpaceSelect = ['Id', 'Name', 'LongDescription'];
 
+/**
+ * Default per-REQUEST timeout (ms). This bounds a single HTTP round-trip, NOT a whole
+ * paginated fetch: the client pages at $top=1000 and each page is its own request, so a
+ * multi-page (multi-minute/hour) fetch is unaffected — only an individual request that
+ * stalls (e.g. queued server-side under load, or hitting an unhealthy endpoint) trips it.
+ * Without this, a stalled request waits forever (axios default timeout = 0) and holds the
+ * caller's slot indefinitely. Generous by design; override per-client or per-call as needed.
+ */
+export const DEFAULT_REQUEST_TIMEOUT_MS = 120_000;
+
+/**
+ * Default timeout (ms) for the login/token request specifically — always a small, quick
+ * call, so a much tighter bound is safe. A hung login is never legitimate.
+ */
+export const DEFAULT_LOGIN_TIMEOUT_MS = 30_000;
+
 export class FamisClient {
   host: string;
   http: AxiosInstance;
@@ -218,12 +235,17 @@ export class FamisClient {
      * here) if a request returns 401 AND the refresh-token attempt also fails. Defaults to false.
      */
     reauthOnFailure?: boolean;
+    /** Per-request timeout (ms) for all subsequent calls. Defaults to DEFAULT_REQUEST_TIMEOUT_MS. */
+    requestTimeoutMs?: number;
+    /** Timeout (ms) for the initial login request. Defaults to DEFAULT_LOGIN_TIMEOUT_MS. */
+    loginTimeoutMs?: number;
   }) {
     const cred = await this.login({
       username: opts.username,
       password: opts.password,
       url: opts.host,
       debug: opts.debug,
+      timeoutMs: opts.loginTimeoutMs,
     });
     if (opts.debug === true) {
       console.log(`Logged in with ${JSON.stringify(cred)}`);
@@ -238,6 +260,7 @@ export class FamisClient {
       opts.reauthOnFailure ?? false,
       // Only retain credentials when the caller explicitly opted in.
       opts.reauthOnFailure ? { username: opts.username, password: opts.password } : undefined,
+      opts.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS,
     );
   }
 
@@ -247,6 +270,8 @@ export class FamisClient {
     debug?: boolean;
     autoRetry?: boolean;
     onComplete?: OnCompleteCallback;
+    /** Per-request timeout (ms) for all calls. Defaults to DEFAULT_REQUEST_TIMEOUT_MS. */
+    requestTimeoutMs?: number;
   }): FamisClient {
     return new FamisClient(
       {
@@ -267,6 +292,9 @@ export class FamisClient {
       opts.debug ?? false,
       opts.autoRetry ?? false,
       opts.onComplete,
+      false,
+      undefined,
+      opts.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS,
     );
   }
 
@@ -275,9 +303,12 @@ export class FamisClient {
     password: string;
     url: string;
     debug?: boolean;
+    /** Timeout (ms) for the login request. Defaults to DEFAULT_LOGIN_TIMEOUT_MS. */
+    timeoutMs?: number;
   }): Promise<LoginResponse> {
     const http = axios.create({
       baseURL: opts.url,
+      timeout: opts.timeoutMs ?? DEFAULT_LOGIN_TIMEOUT_MS,
     });
     if (opts?.debug === true)
       console.log(`Logging into ${opts.url}. ${(opts.username, opts.password)}`);
@@ -333,6 +364,7 @@ export class FamisClient {
     onComplete?: OnCompleteCallback,
     reauthOnFailure: boolean = false,
     reauthCredentials?: { username: string; password: string },
+    requestTimeoutMs: number = DEFAULT_REQUEST_TIMEOUT_MS,
   ) {
     this.credentials = credentials;
     this.host = host;
@@ -344,6 +376,10 @@ export class FamisClient {
       baseURL: host,
       validateStatus: (status) => true,
       maxBodyLength: Infinity,
+      // Per-request timeout so a stalled request fails fast (and axios destroys the
+      // socket, freeing the slot) instead of hanging forever. Per-request, not
+      // per-operation — safe for paginated fetches (each page is its own request).
+      timeout: requestTimeoutMs,
     });
     if (autoRetry) {
       axiosRetry(this.http, {
@@ -1787,6 +1823,14 @@ export class FamisClient {
     };
   }
 
+  /**
+   * Per-request axios config carrying a QueryContext's timeout override, if set.
+   * Empty object otherwise, so the client-level default `timeout` applies.
+   */
+  private timeoutConfig(context?: QueryContext): AxiosRequestConfig {
+    return context?.timeoutMs != null ? { timeout: context.timeoutMs } : {};
+  }
+
   async getAllBatch<T>(
     context: QueryContext,
     type: string,
@@ -1794,7 +1838,7 @@ export class FamisClient {
   ): Promise<void> {
     let top = 1000;
     const url = context.buildPagedUrl(type, top, 0, true);
-    const resp = await this.http.get(url);
+    const resp = await this.http.get(url, this.timeoutConfig(context));
 
     this.throwResponseError(resp);
     const famisResp = resp.data as FamisResponse<T>;
@@ -1815,7 +1859,7 @@ export class FamisClient {
       const url = context.buildPagedUrl(type, top, i * top);
 
       const req = limiter
-        .schedule(() => this.http.get(url))
+        .schedule(() => this.http.get(url, this.timeoutConfig(context)))
         .then((resp: AxiosResponse<FamisResponse<T>>) => {
           this.throwResponseError(resp);
           if (this.debug) {
@@ -1852,7 +1896,7 @@ export class FamisClient {
       if (this.debug) {
         console.log(`Fetching ${url}`);
       }
-      const resp = await this.http.get(url);
+      const resp = await this.http.get(url, this.timeoutConfig(context));
       durationMs += moment(Date.now()).diff(startDate);
       this.throwResponseError(resp);
       const famisResp = resp.data as FamisResponse<T>;
diff --git a/model/request_context.ts b/model/request_context.ts
index 28064b2..df04477 100644
--- a/model/request_context.ts
+++ b/model/request_context.ts
@@ -7,6 +7,13 @@ export class QueryContext {
     top?: number;
     skip?: number;
     orderBy?: string;
+    /**
+     * Per-call request timeout override (ms) for the HTTP requests this context drives.
+     * Overrides the client's default `requestTimeoutMs`. Use for the rare endpoint whose
+     * single (non-paginated) response legitimately runs longer than the default. Paginated
+     * fetches don't need this — each page is a separate, individually-bounded request.
+     */
+    timeoutMs?: number;
 
     setFilter(filter: string | Filter) {
         this.filter = filter.toString();
@@ -38,6 +45,12 @@ export class QueryContext {
         return this;
     }
 
+    /** Per-call HTTP request timeout override (ms). See `timeoutMs`. */
+    setTimeout(timeoutMs: number) {
+        this.timeoutMs = timeoutMs;
+        return this;
+    }
+
     buildApiUrl(entity: string) {
         let urlPath = `MobileWebServices/api/${entity}`;
         return this.addFiltersToUrl(urlPath);
diff --git a/tests/query_context.test.ts b/tests/query_context.test.ts
index a35ee38..343442b 100644
--- a/tests/query_context.test.ts
+++ b/tests/query_context.test.ts
@@ -32,4 +32,15 @@ describe('QueryContext', function () {
         const uri = new URL(`http://example.com/${queryUrl}`);
         expect(uri.searchParams.get('$filter')).toEqual("UserName eq 'O''Brien'");
     });
+    it('carries a per-call timeout override via setTimeout (not part of the URL)', function () {
+        const context = new QueryContext().setSelect('Id').setTimeout(5000);
+        expect(context.timeoutMs).toEqual(5000);
+        // timeout is a request-config concern, never serialized into the OData URL
+        const uri = new URL(`http://example.com/${context.buildUrl('/test')}`);
+        expect(uri.searchParams.get('$select')).toEqual('Id');
+        expect(uri.href).not.toContain('timeout');
+    });
+    it('defaults timeoutMs to undefined so the client default applies', function () {
+        expect(new QueryContext().timeoutMs).toBeUndefined();
+    });
 });
\ No newline at end of file

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions