fix: exit process after dailyRefinementJob finishes#612
fix: exit process after dailyRefinementJob finishes#612RohanExploit wants to merge 1 commit intomainfrom
Conversation
Fixes an issue where `scheduler/dailyRefinementJob.ts` hangs indefinitely when executed with the `--run-now` flag. The script now awaits the refinement job and explicitly calls `process.exit(0)` on success or `process.exit(1)` on error.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for fixmybharat canceled.
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses the scheduler CLI hanging when invoked with --run-now by waiting for the refinement to finish and then terminating the Node process with an explicit exit code.
Changes:
- Await
DailyRefinementJob.runRefinement()when--run-nowis provided. - Exit the process with code
0on success and1on error (intended behavior).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| job.runRefinement().then(() => { | ||
| process.exit(0); | ||
| }).catch((err) => { | ||
| console.error(err); | ||
| process.exit(1); |
There was a problem hiding this comment.
The .catch(...) here likely never runs because runRefinement() catches errors internally and only logs them (it doesn’t rethrow). That means --run-now will still process.exit(0) even when refinement fails. To make the exit code reliable, either let runRefinement() propagate/rethrow errors (or return a success/failure result) and handle it here, instead of swallowing exceptions inside runRefinement().
| process.exit(0); | ||
| }).catch((err) => { | ||
| console.error(err); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
Using process.exit(...) avoids the hang, but it also bypasses normal cleanup (e.g., leaving the sqlite connection and the cron task running until forced termination). A cleaner fix is to skip cron.schedule(...) entirely when --run-now is set, and explicitly close the sqlite DB after the one-off run so the process can exit naturally (or close it before calling process.exit).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scheduler/dailyRefinementJob.ts">
<violation number="1" location="scheduler/dailyRefinementJob.ts:134">
P2: `--run-now` can return a false success exit code because `runRefinement()` swallows errors, so this promise chain almost always resolves and exits with 0.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // For testing/manual trigger, we can also pass a flag or just run immediately if needed | ||
| if (process.argv.includes("--run-now")) { | ||
| job.runRefinement(); | ||
| job.runRefinement().then(() => { |
There was a problem hiding this comment.
P2: --run-now can return a false success exit code because runRefinement() swallows errors, so this promise chain almost always resolves and exits with 0.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scheduler/dailyRefinementJob.ts, line 134:
<comment>`--run-now` can return a false success exit code because `runRefinement()` swallows errors, so this promise chain almost always resolves and exits with 0.</comment>
<file context>
@@ -131,7 +131,12 @@ if (require.main === module) {
// For testing/manual trigger, we can also pass a flag or just run immediately if needed
if (process.argv.includes("--run-now")) {
- job.runRefinement();
+ job.runRefinement().then(() => {
+ process.exit(0);
+ }).catch((err) => {
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scheduler/dailyRefinementJob.ts`:
- Around line 134-139: The .catch() is unreachable because runRefinement()
swallows errors; modify either runRefinement() or this caller: Option A — change
the runRefinement() implementation (the runRefinement function on the job
object) to re-throw the caught error after logging so job.runRefinement()
returns a rejected promise and the caller's .catch() runs; Option B — change
runRefinement() to return a boolean status (true on success, false on failure)
and update this call site to await the result (const ok = await
job.runRefinement()) and then call process.exit(0) for success or
process.exit(1) for failure instead of relying on .catch(); pick one approach
and apply consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67663b29-31b6-48ef-ad75-933f2813fbeb
📒 Files selected for processing (1)
scheduler/dailyRefinementJob.ts
| job.runRefinement().then(() => { | ||
| process.exit(0); | ||
| }).catch((err) => { | ||
| console.error(err); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
.catch() block is unreachable — runRefinement() swallows all errors.
The runRefinement() method (lines 85-87) catches errors internally and logs them but does not re-throw. The promise always resolves, so this .catch() never executes and the process exits with code 0 even on failure.
Either re-throw the error in runRefinement() or return a status to indicate failure:
Option 1: Re-throw in runRefinement()
} catch (error) {
console.error("Error during daily refinement:", error);
+ throw error;
}Option 2: Return success/failure and check it here
- job.runRefinement().then(() => {
- process.exit(0);
- }).catch((err) => {
- console.error(err);
- process.exit(1);
- });
+ job.runRefinement().then((success) => {
+ process.exit(success ? 0 : 1);
+ });And update runRefinement() to return true/false accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scheduler/dailyRefinementJob.ts` around lines 134 - 139, The .catch() is
unreachable because runRefinement() swallows errors; modify either
runRefinement() or this caller: Option A — change the runRefinement()
implementation (the runRefinement function on the job object) to re-throw the
caught error after logging so job.runRefinement() returns a rejected promise and
the caller's .catch() runs; Option B — change runRefinement() to return a
boolean status (true on success, false on failure) and update this call site to
await the result (const ok = await job.runRefinement()) and then call
process.exit(0) for success or process.exit(1) for failure instead of relying on
.catch(); pick one approach and apply consistently.
Fixes an issue where
scheduler/dailyRefinementJob.tshangs indefinitely when executed with the--run-nowflag. The script now awaits the refinement job and explicitly callsprocess.exit(0)on success orprocess.exit(1)on error.PR created automatically by Jules for task 6153720936118833140 started by @RohanExploit
Summary by cubic
Fixes a hang when running
scheduler/dailyRefinementJob.tswith--run-nowby awaiting the refinement job. The script now exits with code 0 on success or 1 on error, preventing runaway processes in manual/CI runs.Written for commit bdcb6fd. Summary will update on new commits.
Summary by CodeRabbit