Skip to content
This repository was archived by the owner on Sep 11, 2025. It is now read-only.

Conversation

@mattjohnsonpint
Copy link
Contributor

small update to #931

@mattjohnsonpint mattjohnsonpint requested review from a team and Copilot July 10, 2025 04:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a guard to avoid nil pointer dereferences when the Sentry client is not initialized in the CaptureError and CaptureWarning functions.

  • Cache the result of hub.Client() in a local variable and check for nil before proceeding
  • Replace repeated hub.Client() calls with the cached client
  • Early return if client is nil
Comments suppressed due to low confidence (2)

runtime/sentryutils/sentry.go:212

  • Add tests covering the scenario where hub.Client() returns nil for both CaptureError and CaptureWarning to ensure these functions exit cleanly without panicking.
	client := hub.Client()

runtime/sentryutils/sentry.go:210

  • [nitpick] Update the function comment to note that CaptureError will return early if the Sentry client is nil, clarifying this behavior for future readers.
func CaptureError(ctx context.Context, err error, msg string, opts ...func(event *sentry.Event)) {

Comment on lines 211 to +212
hub := ActiveHub(ctx)
client := hub.Client()
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting the client retrieval and nil-check into a helper function to avoid duplicating this pattern in both CaptureError and CaptureWarning.

Suggested change
hub := ActiveHub(ctx)
client := hub.Client()
client := getClient(ctx)

Copilot uses AI. Check for mistakes.
@mattjohnsonpint mattjohnsonpint merged commit 480e68d into main Jul 10, 2025
33 checks passed
@mattjohnsonpint mattjohnsonpint deleted the mjp/sentry branch July 10, 2025 04:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants