diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index d3b51b7ba..5c0f2f73c 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -7,10 +7,12 @@ ### New Features and Improvements ### Bug Fixes +* Fix potential panic in `shouldRetry` function by using comma-ok idiom for type assertion ([#1438](https://github.com/databricks/databricks-sdk-go/pull/1438)). ### Documentation ### Internal Changes +* Remove deprecated `rand.Seed` call in retry backoff logic ([#1438](https://github.com/databricks/databricks-sdk-go/pull/1438)). ### API Changes * Add `Force` field for [pipelines.DeletePipelineRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/pipelines#DeletePipelineRequest). \ No newline at end of file diff --git a/retries/retries.go b/retries/retries.go index a0ce9354b..b6cc5b81f 100644 --- a/retries/retries.go +++ b/retries/retries.go @@ -77,7 +77,6 @@ func backoff(attempt int) time.Duration { wait = maxWait } // add some random jitter - rand.Seed(time.Now().UnixNano()) jitter := rand.Intn(int(maxJitter)-int(minJitter)+1) + int(minJitter) wait += time.Duration(jitter) return wait @@ -233,8 +232,8 @@ func shouldRetry(err error) bool { if err == nil { return false } - e := err.(*Err) - if e == nil { + e, ok := err.(*Err) + if !ok || e == nil { return false } return !e.Halt diff --git a/retries/retries_test.go b/retries/retries_test.go index 870518111..baf97d40e 100644 --- a/retries/retries_test.go +++ b/retries/retries_test.go @@ -127,6 +127,23 @@ func TestRetriesNegativeTimeout(t *testing.T) { assert.Equal(t, r.config.Timeout(), time.Duration(-1)) } +func TestShouldRetryWithNonErrType(t *testing.T) { + // Test that shouldRetry handles non-*Err types gracefully without panicking + regularError := errors.New("a regular error") + assert.False(t, shouldRetry(regularError)) + + // Test with nil + assert.False(t, shouldRetry(nil)) + + // Test with *Err that should not retry + haltErr := Halt(errors.New("halt error")) + assert.False(t, shouldRetry(haltErr)) + + // Test with *Err that should retry + continueErr := Continue(errors.New("continue error")) + assert.True(t, shouldRetry(continueErr)) +} + func ExampleNew() { // Default retries for 20 minutes on any error New[int]()