Skip to content

Error handling, pagination support and cleanup#18

Merged
tarmojussila merged 3 commits intomainfrom
bugfix/error-handling-cleanup
Mar 2, 2026
Merged

Error handling, pagination support and cleanup#18
tarmojussila merged 3 commits intomainfrom
bugfix/error-handling-cleanup

Conversation

@tarmojussila
Copy link
Owner

This pull request improves the robustness and reliability of the code that interacts with the Z.ai API and fetches pull request file changes. The main changes include adding pagination support for fetching changed files, introducing response size limits and request timeouts for API calls, and improving error handling for API responses.

API request and response handling improvements:

  • Added a maximum response size limit (MAX_RESPONSE_SIZE) to prevent processing excessively large responses from the Z.ai API, and implemented logic to abort the request if this limit is exceeded.
  • Introduced a request timeout (REQUEST_TIMEOUT_MS) for Z.ai API requests to avoid hanging indefinitely on slow or unresponsive calls.
  • Improved error handling for API responses by catching and reporting invalid JSON responses and truncating error messages for large responses.

Pull request file fetching:

  • Updated the getChangedFiles function to support pagination, ensuring all changed files are fetched even when there are more than 100 files in a pull request.

- Wrap JSON.parse in try/catch for malformed API responses
- Truncate error response bodies to prevent log leakage
- Add 1MB response size limit to prevent memory exhaustion
- Handle pagination for PRs with 100+ changed files
- Add 5-minute timeout on Z.ai API requests to prevent indefinite hangs
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

Z.ai Code Review

The changes improve robustness by adding pagination, timeouts, and memory limits. However, there is a specific logical fragility in the pagination implementation.

1. Pagination Logic Fragility (Bug Risk)

In getChangedFiles, the pagination loop relies on a "magic number" to determine when to stop fetching pages:

per_page: 100,
// ...
if (data.length < 100) break;

If the per_page value in the request options is ever modified (e.g., to 50 or 30 to reduce load), the break condition will be incorrect. This could cause the function to stop prematurely (missing files) or enter an infinite loop (if per_page is increased and the API keeps returning full pages).

Recommendation: Define per_page as a constant or variable and reuse it in the break condition.

const PER_PAGE = 100;
// ...
while (true) {
  const { data } = await octokit.rest.pulls.listFiles({
    // ...
    per_page: PER_PAGE,
    page,
  });
  files.push(...data);
  if (data.length < PER_PAGE) break;
  page++;
}

2. Implicit Buffer Conversion

In callZaiApi, data += chunk concatenates a Buffer to a String. While Node.js handles this implicitly, it can be a source of bugs if the stream encoding is not strictly UTF-8 or if the stream emits empty buffers.

Recommendation: Explicitly convert the chunk to a string for clarity and safety:

data += chunk.toString('utf8');

3. Memory Limit Timing

The check if (data.length > MAX_RESPONSE_SIZE) occurs after concatenation (data += chunk).
If data is currently at the limit (1,048,576 bytes) and a chunk of 1KB arrives, the string will temporarily grow to ~1.05MB before the check triggers the destroy. Given the small margin over 1MB, this is generally acceptable, but strictly speaking, you are allowing the memory limit to be exceeded by the size of one chunk. This is minor but worth noting for strict memory constraints.

@tarmojussila tarmojussila merged commit 2601dd1 into main Mar 2, 2026
1 check passed
@tarmojussila tarmojussila deleted the bugfix/error-handling-cleanup branch March 2, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant