← Back to PRs

#5630: refactor: extract unified withTimeout utility

by lailoo open 2026-01-31 19:09 View on GitHub →
channel: slack
## Summary Extracts a unified `withTimeout` utility function to reduce code duplication across the codebase. ## Changes - Add `src/utils/with-timeout.ts` with configurable timeout and custom error message - Add comprehensive unit tests for `withTimeout` - Add **consistency tests** to verify behavior matches original implementations - Refactor `slack/probe.ts` to use unified `withTimeout` - Refactor `line/probe.ts` to use unified `withTimeout` ## Consistency Verification Created dedicated consistency tests that compare the new implementation against the original inline implementations: | Test Case | Original | New | Status | |-----------|----------|-----|--------| | Normal completion | ✅ | ✅ | Match | | Timeout error message | `"timeout"` | `"timeout"` | Match | | timeoutMs=0 | Returns directly | Returns directly | Match | | timeoutMs=-1 | Returns directly | Returns directly | Match | | Error propagation | ✅ | ✅ | Match | ## Benefits - Reduces code duplication (7+ similar implementations in codebase) - Consistent behavior across all usages - Easier to maintain and test - Other files can be refactored incrementally ## Testing - 7 unit tests for `withTimeout` - 5 consistency tests comparing with original implementation - 128 existing tests in `slack/` and `line/` modules pass <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR introduces a shared `src/utils/with-timeout.ts` helper and updates the Slack and LINE probe flows to use it (passing `"timeout"` to preserve the old error string). It also adds unit tests plus “consistency” tests that compare the new helper against the previous inline implementations. Overall the refactor looks low-risk and the new helper matches the prior behavior for the call sites touched. The main items to watch are (1) `withTimeout`’s `!timeoutMs` check, which can silently treat `NaN` the same as `0` and skip timeouts, and (2) one timing-based unit test that’s likely to be flaky in CI. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minor follow-up tweaks. - The change is a contained refactor with good test coverage and call-site preservation of the legacy timeout error message. Remaining concerns are limited to edge-case input handling (`NaN`/falsy timeout values) and a potentially flaky, timing-based test; neither suggests a broad behavioral regression for typical usage. - src/utils/with-timeout.ts; src/utils/with-timeout.test.ts <!-- greptile_other_comments_section --> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=0d0c8278-ef8e-4d6c-ab21-f5527e322f13)) <!-- /greptile_comment -->

Most Similar PRs