Skip to content

Fix recover() not working in CatchPanic functions#7

Merged
warkanum merged 4 commits intoserverfrom
copilot/sub-pr-5-again
Dec 30, 2025
Merged

Fix recover() not working in CatchPanic functions#7
warkanum merged 4 commits intoserverfrom
copilot/sub-pr-5-again

Conversation

Copy link
Contributor

Copilot AI commented Dec 30, 2025

The CatchPanic and CatchPanicCallback functions call recover() directly in their body, but recover() only works when called from within a deferred function. This renders panic recovery non-functional.

Changes

  • Modified function signatures: Both functions now return func() that must be invoked
  • Extracted context early: Context extraction moved outside closure to avoid capturing args slice
  • Updated call sites: Added trailing () to all usages in pkg/security/provider.go
  • Updated documentation: Fixed examples in pkg/errortracking/README.md

Usage

// Before (non-functional)
defer logger.CatchPanic("MyFunction")

// After (functional)
defer logger.CatchPanic("MyFunction")()

// With callback
defer logger.CatchPanicCallback("MyFunction", func(err any) {
    // cleanup
})()

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 30, 2025 11:06
…ction

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>
Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>
Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP address feedback from review on Server Manager feature PR Fix recover() not working in CatchPanic functions Dec 30, 2025
Copilot AI requested a review from warkanum December 30, 2025 11:09
Copy link
Member

@warkanum warkanum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make sure everywhere we use CatchPanicCallback and logger.CatchPanic we call the inner function

Copy link
Contributor Author

Copilot AI commented Dec 30, 2025

Just make sure everywhere we use CatchPanicCallback and logger.CatchPanic we call the inner function

Confirmed - all usages are already calling the inner function with ():

  • pkg/security/provider.go:299: defer logger.CatchPanic("ApplyColumnSecurity")()
  • pkg/security/provider.go:440: defer logger.CatchPanic("GetRowSecurityTemplate")()

The README documentation also shows the correct usage pattern with () at the end.

Copy link
Member

@warkanum warkanum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@warkanum warkanum marked this pull request as ready for review December 30, 2025 11:29
@warkanum warkanum merged commit 7e1718e into server Dec 30, 2025
1 check passed
@warkanum warkanum deleted the copilot/sub-pr-5-again branch December 30, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants