Skip to content
PR Quorum
Sign inStart free
← Review gallery
grafana/grafana #128724openreviewed 6h ago

Provisioning: retry dashboard provisioning while the folder API is unavailable

by filewalkwithme+240 36 filesreviewed in 17sView PR on GitHub
needs attentionThe panel surfaced issues that deserve attention before merge.2 findings · 1 high+

Reviewer panel

Correctness
deepseek/deepseek-v4-flash
1 found
Security
deepseek/deepseek-v4-flash
0 found
Architecture
deepseek/deepseek-v4-flash
1 found

Findings

01Hhighconf 95%

isRetriableFolderError may miss wrapped gRPC status errors

pkg/services/provisioning/provisioning.go:186

The function isRetriableFolderError checks status.Code(err) directly, but status.Code returns codes.Unknown for errors that are not a gRPC status or are wrapped with fmt.Errorf("...: %w", err). The gRPC status library's FromError helper is the correct way to unwrap the error chain. If the underlying gRPC status error is wrapped (e.g. fmt.Errorf("%w: %w", dashboards.ErrGetOrCreateFolder, status.Error(...))), status.Code(err) will return codes.Unknown and the retry will not trigger, defeating the purpose of the change. The test at line 117 wraps the gRPC error with fmt.Errorf("%w: %w", ...) which happens to work because status.Code does unwrap one level via errors.Unwrap, but deeper wrapping or different wrapping patterns would break.

02Llowconf 80%

time.After timer not stopped on context cancellation

pkg/services/provisioning/provisioning.go:204

In provisionDashboardsWithRetry, time.After creates a timer that is not explicitly stopped when the context is cancelled. If the context is cancelled during the backoff, the function returns immediately but the timer remains allocated until it fires (up to 15 seconds). This is a minor resource leak. Consider using time.NewTimer and stopping it on cancellation to free the timer resource promptly.

Want this on your own pull requests?
The same reviewer panel runs on every PR — one focused review, no noise.
Start freeLive demo