ApiError crashes on network errors (ECONNRESET) — getters deref this.resp unguarded → 'reading status'

Summary

A request that fails with a network error (e.g. ECONNRESET) — where axios rejects with no response — surfaces to callers as:

Cannot read properties of undefined (reading 'status')

instead of the real error. This masks the underlying network failure and makes diagnosis misleading.

Where it's been hit

The FAMIS connector's nightly asset cache (getAllAssetsBatch) has hit this on two consecutive fleet runs (2026-07-09 and 2026-07-10), same tenants each time (udst, udstsso — large asset sets, ~241K), when a mid-stream page fetch is reset. The loader fails with the cryptic reading 'status' message rather than a clear ECONNRESET, so the tenant's build fails and the real cause is hidden. Observed with facility360@1.2.0.

Root cause — `ApiError` dereferences `this.resp` unguarded (`errors.ts`)

export class ApiError extends Error {
  private resp: AxiosResponse;
  constructor(resp: AxiosResponse) { super(); this.resp = resp; ... }

  get isAuthorizationError(): boolean {
    return this.resp.status == 401 || this.resp.status === 403;   // ← this.resp may be undefined
  }
  get message(): string {
    try { if (this.resp.data.Message) return this.resp.data.Message; } catch (e) {}
    return this.resp.statusText;                                  // ← unguarded
  }
  toString() { ... this.resp.request ... this.resp.config.baseURL ... }  // ← unguarded
}

On a network error there is no AxiosResponse. When an ApiError is constructed/propagated without a response, reading any getter (isAuthorizationError, message, toString) throws Cannot read properties of undefined (reading 'status'/'statusText'/...). That TypeError then becomes the surfaced error, hiding the ECONNRESET.

(The connector worked around this for the auth path only — reading resp?.status defensively in its #421 change — but the root fix belongs here so all call paths, including getAllAssetsBatch, are covered and the workaround can be retired.)

Proposed fix

1. Make ApiError null-safe (defensive — covers every call path):

get isAuthorizationError(): boolean {
  return this.resp?.status === 401 || this.resp?.status === 403;
}

get message(): string {
  if (!this.resp) {
    return (this as Error).cause instanceof Error
      ? (this as any).cause.message
      : 'FAMIS API error (no HTTP response — network error?)';
  }
  try { if (this.resp.data?.Message) return this.resp.data.Message; } catch {}
  return this.resp.statusText ?? 'FAMIS API error';
}

toString() {
  const message = this.message;
  if (this.isAuthorizationError && !message) return 'Authorization Failed';
  if (this.resp?.request) {
    return `${message}\nMETHOD: ${this.resp.request.method}\nHOST: ${this.resp.config?.baseURL}\nURL: ${this.resp.request.path}\n`;
  }
  return message;
}

2. Don't turn a response-less error into an ApiError (the real root): at the point where an axios error is converted, only build ApiError when err.response exists; otherwise rethrow the original AxiosError (it already carries code: 'ECONNRESET' and a meaningful message) or wrap it in a distinct NetworkError. Optionally preserve the cause: super(); (this as any).cause = originalError;.

3. Regression test (mirrors the connector's cache.loader.service.spec regression):

it('does not throw when constructed without a response (network error)', () => {
  const e = new ApiError(undefined as any);
  expect(() => e.isAuthorizationError).not.toThrow();
  expect(() => e.message).not.toThrow();
  expect(() => e.toString()).not.toThrow();
  expect(e.isAuthorizationError).toBe(false);
});

Impact / priority

Medium. Not data-corrupting (the connector preserves the prior cache and retries), but it (a) fails the affected loaders each run for ≥2 tenants, and (b) hides the true cause (ECONNRESET) behind a phantom reading 'status' TypeError, costing debugging time. Recurs every fleet run.

References

Connector run reports: docs/operations/famis-full-run-results-2026-07-09.md, -2026-07-10.md (facility360_r7).
Connector workaround for the auth path: PR #421 (isAuthRejection/safeErrMsg).

Update after reading `getAllBatch` — exact crash line + a parameterization ask

Tracing the actual code (famis_client.ts, getAllBatch<T> — the fn getAllAssetsBatch delegates to) pins this down further.

1. Exact crash origin

The parallel-page .catch passes a non-null-asserted response into throwResponseError:

// getAllBatch, ~line 1826
.catch((e: AxiosError) => {
  this.throwResponseError(e.response!);   // ← on ECONNRESET, e.response is UNDEFINED
});

// ~line 1929
throwResponseError(resp: AxiosResponse) {
  if (resp.status !== 200) {              // ← undefined.status → "Cannot read properties of undefined (reading 'status')"
    throw new ApiError(resp);
  }
}

On a network reset (pages 2..N of the batch) e.response is undefined; e.response! lies to the type system, and throwResponseError then reads .status on undefined. Fix: in the .catch, if e.response is missing, rethrow the original AxiosError (it carries code: 'ECONNRESET' + a real message) or a NetworkError — don't fabricate an ApiError from undefined. (The null-safe ApiError getters from the original description are still worth doing as defense-in-depth.)

2. Page size & concurrency are hardcoded — please make them parameters

// getAllBatch
let top = 1000;                                        // ~line 1795 — hardcoded; getAllBatch does NOT read context.top
const limiter = new Bottleneck({ maxConcurrent: 4 });  // ~line 1813 — hardcoded

getAllBatch (unlike getAllPaged, which honors context.top) ignores QueryContext.top, so a caller cannot lower the assets page size or fan-out today. The udst/udstsso assets failures (~241K assets, 6-way $expand) are almost certainly the heavy per-page query tripping a backend reset — see the connector-side finding: the legacy accruent-tasks nightly runs all 9 reference loaders for udst/udstsso with zero failures; only the connector's new assets fetch resets. So the resolution is to dial the assets fetch down for the big tenants, and that needs to be tunable from the caller.

Requested change — make both caller-settable, defaults unchanged (backward-compatible):

getAllAssetsBatch(
  context: QueryContext,
  callback: ResultCallback<Asset>,
  opts?: { pageSize?: number; maxConcurrent?: number },
): Promise<void>

// getAllBatch:
const top = opts?.pageSize ?? 1000;
const limiter = new Bottleneck({ maxConcurrent: opts?.maxConcurrent ?? 4 });

Why parameters (vs just lowering the defaults): the right values are empirical — we'll need to try e.g. pageSize 500 → 250 and maxConcurrent 4 → 2 against the large tenants. With these exposed, the connector tunes via its own env/config and iterates without an SDK release per attempt, and can apply small pages/low concurrency only to the few huge-asset tenants while leaving everyone else on the fast path. Keeping 1000/4 as defaults means no impact to other SDK consumers.

The two levers do different work, so both matter: pageSize↓ shrinks each request's $expand payload (likely the bigger win against the reset); maxConcurrent↓ reduces simultaneous pressure on the FAMIS backend.

Suggested scope for this issue

Fix the e.response! crash (rethrow the real network error) — correctness.
Add pageSize + maxConcurrent params to getAllBatch/getAllAssetsBatch, defaults 1000/4.
(Nice-to-have) exponential backoff on the network-error retry (currently retries: 2, retryDelay: () => 2).

Items 1+2 together let us actually resolve the udst/udstsso net-failures by tuning the fetch from the connector.

Fixed in 1.3.0 (PR #110): the response-less network-error path no longer builds an ApiError from an undefined response — isAuthorizationError/toString are null-safe and the real error (e.g. ECONNRESET) is rethrown so the true cause surfaces. Published and running in prod (via 1.4.0). Closing.

Navigation Menu

Repository navigation

ApiError crashes on network errors (ECONNRESET) — getters deref this.resp unguarded → 'reading status' #109

Summary

Where it's been hit

Root cause — `ApiError` dereferences `this.resp` unguarded (`errors.ts`)

Proposed fix

Impact / priority

References

Update after reading `getAllBatch` — exact crash line + a parameterization ask

1. Exact crash origin

2. Page size & concurrency are hardcoded — please make them parameters

Suggested scope for this issue

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ApiError crashes on network errors (ECONNRESET) — getters deref this.resp unguarded → 'reading status' #109

Description

Summary

Where it's been hit

Root cause — ApiError dereferences this.resp unguarded (errors.ts)

Proposed fix

Impact / priority

References

Activity

alphahlee commented on Jul 10, 2026

Update after reading getAllBatch — exact crash line + a parameterization ask

1. Exact crash origin

2. Page size & concurrency are hardcoded — please make them parameters

Suggested scope for this issue

alphahlee commented on Jul 23, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Root cause — `ApiError` dereferences `this.resp` unguarded (`errors.ts`)

Update after reading `getAllBatch` — exact crash line + a parameterization ask