fix: 1243 returns both resetErr and the request's err on retries.Halt

[pengux]

What changes are proposed in this pull request?

This fixes the issue with the request error being discarded when the request body is not able to be reset.

How is this tested?

Test are added to assert that both errors (resetErr and request's err) are wrapped in the returned error.

Fixes #1243

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 15 days if no further activity occurs. If this PR is still relevant, please leave a comment or push new changes to keep it open. Thank you for your contributions.

[pengux]

Still relevant

[renaudhartert-db]

Hi @pengux, sorry for the late response on this.

The fix makes sense, the original request error should not be silently discarded when the body can't be reset. One small suggestion on the error message: I think the original request error should come first since that's the one the caller cares about, with the reset failure as secondary context:

fmt.Errorf("%w (unable to reset request body for retry: %w)", err, resetErr)

This way errors.Is matches the request error first, and the message reads more naturally.

The changelog entry also needs to follow the format used in NEXT_CHANGELOG.md -- it should be a bullet point with a short description, e.g.:

* Fixed request error being discarded when the request body cannot be reset for retry ([#1363](https://github.com/databricks/databricks-sdk-go/pull/1363)).

Let us know if you'd like to make these changes, or if you'd prefer we take the PR over from here and land it. Either way works for us.

[pengux]

Hi @pengux, sorry for the late response on this.

The fix makes sense, the original request error should not be silently discarded when the body can't be reset. One small suggestion on the error message: I think the original request error should come first since that's the one the caller cares about, with the reset failure as secondary context:

fmt.Errorf("%w (unable to reset request body for retry: %w)", err, resetErr)

This way errors.Is matches the request error first, and the message reads more naturally.

The changelog entry also needs to follow the format used in NEXT_CHANGELOG.md -- it should be a bullet point with a short description, e.g.:

* Fixed request error being discarded when the request body cannot be reset for retry ([#1363](https://github.com/databricks/databricks-sdk-go/pull/1363)).

Let us know if you'd like to make these changes, or if you'd prefer we take the PR over from here and land it. Either way works for us.

@renaudhartert-db Thanks for reviewing, I've updated the PR according to your feedback now.

[github-actions]

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-go

Inputs:

• PR number: 1363
• Commit SHA: a9de7f68d28ce29a6f5bd534ed7a0149a9a60d84

Checks will be approved automatically on success.