Skip to content

Add request timeout, push retry and XML-sanitizing fallback for feed parsing#4

Merged
hoangsvit merged 2 commits into
mainfrom
codex/optimize-data-processing-time
Apr 15, 2026
Merged

Add request timeout, push retry and XML-sanitizing fallback for feed parsing#4
hoangsvit merged 2 commits into
mainfrom
codex/optimize-data-processing-time

Conversation

@hoangsvit

Copy link
Copy Markdown
Member

Motivation

  • Improve reliability and performance of feed fetching by adding per-request timeouts and clearer retry behaviour.
  • Recover from non-well-formed XML feeds by sanitizing and retrying parsing without causing unnecessary network retries.
  • Handle non-fast-forward git push races from concurrent workflow runs by retrying pushes and rebasing on the latest remote.

Description

  • Added new inputs request_timeout and push_retry_count to action.yml and documented a new "Performance tuning" section in README.md.
  • Implemented per-feed HTTP timeout and AbortController-based cancellation using REQUEST_TIMEOUT, and added logging of estimated worst-case wait times in src/blog-post-workflow.js.
  • Replaced direct parser.parseURL calls with fetchFeedXml + parser.parseString, added sanitizeXml fallback and an isXmlParseError/shouldRetryFeedRequest policy so only network/HTTP errors trigger retries.
  • Added a push retry loop in src/utils.js/dist/blog-post-workflow.js that will attempt git push up to push_retry_count times and run git pull --rebase on failures to recover from concurrent upstream updates.

Testing

  • Built the project bundle with npm run build and confirmed dist/blog-post-workflow.js was updated successfully.
  • No unit tests were added or modified as part of this change.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd8ed4b8a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/blog-post-workflow.js
throw new Error(`Status code ${response.status}`);
}
const xmlText = await response.text();
return response.text();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep timeout active while reading response body

fetchFeedXml returns response.text() without awaiting it, so the finally block clears the abort timer as soon as headers arrive. For feeds that send headers quickly but stall while streaming the body, request_timeout no longer applies and the workflow can block far longer than configured. Await the body read (or otherwise delay clearTimeout) so the timeout covers the full download.

Useful? React with 👍 / 👎.

Comment thread src/utils.js
Comment on lines +106 to +107
const pushRetryCount =
Number.parseInt(core.getInput('push_retry_count'), 10) || 3;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor explicit zero for push retry count

Using Number.parseInt(..., 10) || 3 makes an explicit push_retry_count: 0 impossible, because 0 is treated as falsy and replaced with 3. In that case the action still performs pull/rebase retry cycles and up to four push attempts, which contradicts the input semantics and prevents disabling retries.

Useful? React with 👍 / 👎.

@hoangsvit hoangsvit merged commit ab672cb into main Apr 15, 2026
2 checks passed
@hoangsvit hoangsvit deleted the codex/optimize-data-processing-time branch April 15, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant