fetchAllPages silently returns an empty list when the response is not an array #5

New issue

Closed

opened 2026-07-29 05:59:45 +00:00 by trtmn · 0 comments

trtmn commented

2026-07-29 05:59:45 +00:00

Owner

Problem

fetchAllPages in src/issue-helpers.js assumes a successful response body is always an array:

if (!res.data.length) break;
items.push(...res.data);
if (res.data.length < 50) break;

If the endpoint returns HTTP 200 with a non-array body (an object, or an HTML page that happened to carry a JSON content type - plausible here since git.trtmn.io sits behind Cloudflare Access and can serve a login page), then:

res.data.length is undefined, so !undefined is true and the loop breaks immediately
the function returns []
resolveLabelIds then reports every requested label as "not found ... skipped"
create_issue succeeds with no labels attached and only a soft warning

So a transport or auth problem is misreported as "your label names are wrong". Worst case, res.data.push on a string body would throw a confusing spread error instead.

Suggested fix

Validate the shape before use and fail loudly:

if (!Array.isArray(res.data)) {
  throw new Error(
    `Expected an array from ${path} but got ${typeof res.data} - the instance may have returned an auth or error page.`
  );
}

Keep the existing MAX_PAGES guard. Consider also surfacing when MAX_PAGES is hit, since silently truncating at 500 items would produce the same class of misleading "not found" warning.

Severity

Low - but it converts an infrastructure failure into a misleading user-facing message, which costs debugging time.

## Problem `fetchAllPages` in `src/issue-helpers.js` assumes a successful response body is always an array: ```js if (!res.data.length) break; items.push(...res.data); if (res.data.length < 50) break; ``` If the endpoint returns HTTP 200 with a non-array body (an object, or an HTML page that happened to carry a JSON content type - plausible here since git.trtmn.io sits behind Cloudflare Access and can serve a login page), then: - `res.data.length` is `undefined`, so `!undefined` is true and the loop breaks immediately - the function returns `[]` - `resolveLabelIds` then reports every requested label as "not found ... skipped" - `create_issue` succeeds with no labels attached and only a soft warning So a transport or auth problem is misreported as "your label names are wrong". Worst case, `res.data.push` on a string body would throw a confusing spread error instead. ## Suggested fix Validate the shape before use and fail loudly: ```js if (!Array.isArray(res.data)) { throw new Error( `Expected an array from ${path} but got ${typeof res.data} - the instance may have returned an auth or error page.` ); } ``` Keep the existing `MAX_PAGES` guard. Consider also surfacing when `MAX_PAGES` is hit, since silently truncating at 500 items would produce the same class of misleading "not found" warning. ## Severity Low - but it converts an infrastructure failure into a misleading user-facing message, which costs debugging time.

trtmn added the

bug

reliability

labels

2026-07-29 06:02:02 +00:00

trtmn referenced this issue from a commit

2026-07-29 13:55:57 +00:00

Fix code-review findings: path encoding, string bodies, timeouts

trtmn referenced this issue

2026-07-29 13:56:39 +00:00

Fix code-review findings: path encoding, string bodies, timeouts #6

trtmn