-
Notifications
You must be signed in to change notification settings - Fork 561
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
handle crashes in goroutine events #1857
Conversation
WalkthroughThe pull request focuses on improving error handling and control flow in the GitHub event processing functions within the Changes
Sequence DiagramsequenceDiagram
participant GH as GitHub Webhook
participant Controller as DiggerController
participant CIBackend as CI Backend
GH->>Controller: Send Pull Request/Issue Comment Event
Controller->>Controller: Panic Recovery Mechanism
alt Valid Event
Controller->>Controller: Validate Event Conditions
Controller->>CIBackend: Process Event
else Invalid Event
Controller->>Controller: Log and Ignore
end
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ No security or compliance issues detected. Reviewed everything up to a28afc9. Security Overview
Detected Code Changes
Diagnostics
Reply to this PR with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/controllers/github.go (2)
312-317
: Include stack traces in panic recovery logs for better debuggingCurrently, the panic recovery in
handlePullRequestEvent
logs only the recovered value, which may not provide enough context for debugging. Including a stack trace will help in identifying the source of the panic more effectively.Apply this diff to include stack traces:
+import "runtime/debug" ... defer func() { if r := recover(); r != nil { - log.Printf("Recovered from panic in handlePullRequestEvent handler: %v", r) + log.Printf("Recovered from panic in handlePullRequestEvent handler: %v\nStack Trace:\n%s", r, debug.Stack()) } }()
693-698
: Include stack traces in panic recovery logs for better debuggingSimilarly, in
handleIssueCommentEvent
, including a stack trace in the panic recovery log will enhance debugging capabilities by providing more detailed context.Apply this diff to include stack traces:
+import "runtime/debug" ... defer func() { if r := recover(); r != nil { - log.Printf("Recovered from panic in handleIssueCommentEvent handler: %v", r) + log.Printf("Recovered from panic in handleIssueCommentEvent handler: %v\nStack Trace:\n%s", r, debug.Stack()) } }()
* add flag to ignore all external directories per project (#1851) * add flag to ignore all external directories per project * revert includeparentblocks flag (#1852) * improve efficiency in terragrunt generation (#1854) * improve efficiency in terragrunt generation * Update action.yml (#1856) * handle crashes in goroutine events (#1857) * fix/recover from webhook goroutines (#1858) * handle crashes in goroutine events * include stacktrace in errors * wip generation of terraform code from application code (#1855) * terraform code generation demo --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: incorrect comment in backend/migrations (#1860) * Update setup-opentofu to fix issues with 1.6.x downloads (#1861) * restructure docs to have no columns (#1862) * Create curl_bootstrap.sh * refactor codegen parts (#1863) * refactor codegen parts * publish ai summaries (#1864) * publish ai summaries * add a header for summary comment (#1865) * fix heading (#1866) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aldo <[email protected]> Co-authored-by: Hugo Samayoa <[email protected]>
at the moment our webhook handler spawns a goroutine event. However if there is a panic in this goroutine it seems that it causes a crash of the gonic, this pr adds a recovery logic in both handling issue comment and pull request goroutines.
Summary by CodeRabbit
New Features
Bug Fixes