From 4404a7f1f60c6c2eecfd56fb410f649e120eb237 Mon Sep 17 00:00:00 2001 From: Nepomuk Crhonek <105591323+Nepomuk5665@users.noreply.github.com> Date: Fri, 23 Jan 2026 14:41:56 +0100 Subject: [PATCH 1/2] Fix panic in shouldRetry type assertion and remove deprecated rand.Seed - Use comma-ok idiom in shouldRetry to handle non-*Err error types safely - Remove deprecated rand.Seed call that was re-seeding on every backoff - Add test coverage for shouldRetry with various error types The shouldRetry function previously used a direct type assertion that would panic if passed an error not of type *Err. While this function is internal and typically receives *Err values, the type assertion should be safe to handle unexpected inputs. The rand.Seed call has been deprecated since Go 1.20 as the global random number generator is now automatically seeded. Additionally, calling Seed on every retry attempt was wasteful. --- retries/retries.go | 5 ++--- retries/retries_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) 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]() From 0cefa2609c108943d86c68300e2cce5ca343f817 Mon Sep 17 00:00:00 2001 From: Nepomuk Crhonek <105591323+Nepomuk5665@users.noreply.github.com> Date: Fri, 23 Jan 2026 16:08:53 +0100 Subject: [PATCH 2/2] Update NEXT_CHANGELOG.md with bug fix and internal change entries --- NEXT_CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) 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