Skip to content

fix: recover sync loop after transient Sync API timeout (DX-9488)#112

Open
harshitha-cstk wants to merge 4 commits into
fix/beta-2.4.0from
fix/dx-9488-sync-loop-recovery-after-timeout
Open

fix: recover sync loop after transient Sync API timeout (DX-9488)#112
harshitha-cstk wants to merge 4 commits into
fix/beta-2.4.0from
fix/dx-9488-sync-loop-recovery-after-timeout

Conversation

@harshitha-cstk

@harshitha-cstk harshitha-cstk commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem

DataSync could permanently stop processing webhook events after a single transient network timeout to the Contentstack Sync API. The webhook endpoint kept returning HTTP 200, but no further content was synced until a manual restart. This is the recurring symptom behind DX-3769 (reopened) and DX-7876 — the retry work in 2.4.0-beta.5 hardened the network layer but did not close the underlying recovery defect.

Root cause

Traced from customer prod logs against the source. A transient ETIMEDOUT on the /sync request escapes as an unhandled rejection → the process-level lockdown fires → after 10s it calls unlock(). But:

  1. Recovery never re-armed the sync gate. unlock() was called without refire=true, so it cleared lockdown but left flag.WQ=false / flag.SQ unreset. check()'s (!SQ && WQ) gate stayed shut → sync never resumed.
  2. Several failure paths wedged flag.SQ=true without ever hitting the lockdown handler (non-network content-type schema errors, filterItems/plugin errors, checkpoint-save failures) → silent permanent freeze.
  3. api.ts double-settled. On socket timeout it both reject()ed and (via destroy()'error') launched a detached retry whose result landed on an already-settled promise and was discarded — so even a successful retry dropped the data and never advanced the checkpoint.
  4. Two recovery invocations were uncaught (cool-off emitter.on('check'), post-reconnect poke()), so their rejections could trip the lockdown.

Changes

File Change
src/core/process.ts Recovery calls unlock(true) instead of unlock()
src/core/index.ts unlock() fully re-arms the gate (reset SQ, set WQ, replay + clear requestCache); check() catch always releases the gate (SQ=false) on any error; emitter.on('check') wrapped with .catch
src/core/inet.ts Post-reconnect poke() wrapped with .catch
src/api.ts Settle-once guards (resolveOnce/rejectOnce); socket timeout routed through the in-place retry so a successful retry resolves the original request

Testing

  • npm run build-ts — compiles cleanly (TypeScript 4.9.5).
  • Jest suite — no regressions: identical pass/fail set with and without these changes (pre-existing failures are unrelated async-teardown leaks in the suite). Relevant suites pass: test/core/sync-resume.ts, test/core/sync.ts, test/core/inet.ts, test/core/q.ts.
  • Reproduction (manual): with TIMEOUT lowered to force an ETIMEDOUT, sync now resumes on the next /notify instead of freezing.

Risk / backward compatibility

  • No config or API changes; no behavioural change on the success path.
  • Behavioural change limited to error recovery: sync now self-heals after a transient timeout instead of stopping.

Checklist

  • Builds locally
  • No new test failures
  • Reproduction validated end-to-end via boilerplate (pending)

🤖 Generated with Claude Code

@harshitha-cstk harshitha-cstk requested a review from a team as a code owner July 13, 2026 11:49
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 0 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens DataSync’s recovery behavior so transient Sync API/network failures don’t permanently wedge the sync loop, focusing on re-arming the sync gate and preventing unhandled rejections during recovery paths.

Changes:

  • Re-arms the sync gate on process-level recovery (unlock(true)), and resets gate flags / replays cached requests on unlock.
  • Ensures sync-gate release on check() failures and wraps event-driven recovery calls with .catch() to avoid unhandled rejections.
  • Fixes src/api.ts request lifecycle to settle exactly once and to retry timeouts “in place” so a successful retry advances checkpoints.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/core/process.ts Uses unlock(true) during unhandled error recovery to re-arm the sync gate.
src/core/inet.ts Wraps post-reconnect poke() with a catch to prevent unhandled rejections.
src/core/index.ts Re-arms gate inside unlock(refire); releases SQ on check() failure; wraps emitter.on('check') invocation.
src/api.ts Adds settle-once guards and routes socket timeouts through in-place retries to avoid dropped successful retries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/index.ts
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 0 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

✅ BUILD PASSED - All security checks passed

unlock() returned check()/fire(), which reject on a sync failure. Callers
(process.ts, q.ts) invoke unlock() fire-and-forget, so that rejection could surface as an unhandled rejection and re-trip the process-level lockdown, undoing the recovery this PR adds. Attach a .catch to both the requestCache replay and the check() call so unlock() handles/logs its own errors and is a safe, self-contained gate toggle.
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 0 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/api.ts
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 0 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

✅ BUILD PASSED - All security checks passed

@aniket-shikhare-cstk aniket-shikhare-cstk 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.

LGTM

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.

4 participants