add date formatting utility - #256
Conversation
|
Deployment failed with the following error: Learn More: https://vercel.com/born-slippys-projects?upgradeToPro=build-rate-limit |
|
Deployment failed with the following error: Learn More: https://vercel.com/born-slippys-projects?upgradeToPro=build-rate-limit |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| let lastError: unknown; | ||
| for (let i = 0; i < opts.retries; i++) { | ||
| try { | ||
| return fn(); |
There was a problem hiding this comment.
CRITICAL: fn() is not awaited, so async rejections are never caught and retried
fn returns a Promise<T>. Calling return fn() without await returns the pending promise immediately; if fn() rejects later, that rejection is not caught by the catch block (which only catches synchronous throws). As a result the retry loop never re-runs for failed async calls, defeating the purpose of withRetry. Await it so rejections are caught:
| return fn(); | |
| return await fn(); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| for (let i = 0; i < opts.retries; i++) { | ||
| try { | ||
| return fn(); | ||
| } catch (e) { |
There was a problem hiding this comment.
WARNING: catch (e) is unreachable for the intended async errors
Because the rejected promise from fn() is not awaited (see line 10), this catch only fires on synchronous throws. Once line 10 is fixed with await, this becomes reachable. Combined with the missing await, the retry logic currently never executes for real failures.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| @@ -0,0 +1,16 @@ | |||
| const DEFAULTS = { | |||
| timeout: 5000, | |||
There was a problem hiding this comment.
SUGGESTION: timeout in DEFAULTS is never used
opts.timeout is never referenced anywhere in the function, so the timeout: 5000 config is dead. Either implement a timeout (e.g. wrap fn() in Promise.race with a timeout, or remove it to avoid misleading callers. Given the PR title mentions no timeout behavior, removal is cleaner.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| lastError = e; | ||
| } | ||
| } | ||
| throw lastError; |
There was a problem hiding this comment.
SUGGESTION: throw lastError can throw undefined when retries is 0
If opts.retries is 0, the loop never runs, lastError stays undefined, and throw lastError throws undefined (not a useful error). Guard for this, e.g. throw lastError ?? new Error("withRetry: no attempts made").
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (1 files)
Fix these issues in Kilo Cloud Reviewed by hy3-20260706:free · Input: 28.4K · Output: 2.2K · Cached: 83.2K |
1 participant
Auto-generated: add date formatting utility
Project: mercato