fix: capture Run() error in metrics controller goroutine#330
Conversation
The goroutine was referencing the outer 'err' variable from metrics.NewController() instead of capturing the return value from metricsController.Run(). By the time the goroutine executes, the outer err is nil (NewController succeeded), so the log line always printed a nil error — making failures impossible to diagnose. Fixes openkruise#325
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #330 +/- ##
=======================================
Coverage 40.76% 40.76%
=======================================
Files 112 112
Lines 12544 12544
=======================================
Hits 5114 5114
Misses 7019 7019
Partials 411 411
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
hey @furykerry can you check out on this pr? |
What this PR does
Fixes a closure bug where the metrics controller goroutine logged the wrong error on failure.
Problem
In
main.go(lines 280-285), the goroutine that runs the metrics controller never captures the return value ofmetricsController.Run(). Instead, it references the outererrvariable from themetrics.NewController()call:Since NewController succeeded (otherwise we'd have exited), err is nil by the time the goroutine runs. So if Run() fails, the log prints a nil error — making the failure impossible to diagnose.
Fix
Capture the return value of Run() into its own variable:
fixes #325