-
-
Notifications
You must be signed in to change notification settings - Fork 838
fix(run-engine): waitpoint update misleading error logs #2566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The `completeWaitpoint` method ignores Prisma update errors of type `P2025` to handle concurrent attempts to complete a waitpoint token. We have an error hook on the Prisma client which logs these errors though. This PR switches to `updateMany` which doesn't throw an error if the waitpoint is in an unexpected state. The behavior remains otherwise unchanged. We currently continue even if the waitpoint is not in `PENDING`, which could lead to race conditions in certain corner cases, though very unlikely. We added a log line to see how often concurrent completion attempts happen.
|
WalkthroughThe system modifies waitpoint completion logic to use an updateMany operation instead of a single-record update. Variable names are updated accordingly. Error handling is simplified: any update error is logged and rethrown; the previous P2025-specific handling is removed. If updateResult.count is 0, it logs that a non-pending waitpoint was targeted. The code then fetches the waitpoint by id in all cases and retains validations for existence and completed status. Error messages are generalized to "Waitpoint not found" and "Waitpoint not completed," and control flow now relies on updateMany results. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)internal-packages/run-engine/src/engine/systems/waitpointSystem.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
🔇 Additional comments (1)
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 |
The
completeWaitpoint
method ignores Prisma update errors of typeP2025
tohandle concurrent attempts to complete a waitpoint token. We have an
error hook on the Prisma client which logs these errors though.
This PR switches to
updateMany
which doesn't throw an error if thewaitpoint is in an unexpected state. The behavior remains otherwise
unchanged. We currently continue even if the waitpoint is not in
PENDING
,which could lead to race conditions in certain corner cases, though very
unlikely. We added a log line to see how often concurrent completion
attempts happen.