Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic in agent #3024

Merged
merged 1 commit into from
Aug 30, 2021
Merged

Fix panic in agent #3024

merged 1 commit into from
Aug 30, 2021

Conversation

dperny
Copy link
Collaborator

@dperny dperny commented Aug 24, 2021

Previously, there was an issue where the agent could panic while attempting to determine if an error was temporary.

Before the change, in agent/exec/errors.go, the function IsTemporary attempted to drill down to a root cause by iterating through the causes of an error by calling errors.Cause. If an error has no cause, then errors.Cause returns that same error.

The issue is that somewhere in the depths of some code, it was possible for the error to have an underlying type that was non-comparable; for example, maps and slices are uncomparable types. This would cause a panic, as the uncomparable type cannot be compared even to itself.

However, one can see that errors.Cause has its own loop, and drills down to the root cause in its own way. There is no need for us to iterate here.

Instead, we can just take a look at the error itself, and then take a look at its cause once. If neither is temporary, the error is not temporary, and we have nothing to worry about.

Previously, there was an issue where the agent could panic while
attempting to determine if an error was temporary.

Before the change, in `agent/exec/errors.go`, the function `IsTemporary`
attempted to drill down to a root cause by iterating through the causes
of an error by calling `errors.Cause`. If an error has no cause, then
`errors.Cause` returns that same error.

The issue is that somewhere in the depths of some code, it was posssible
for the error to have an underlying type that was non-comparable; for
example, maps and slices are uncomparable types. This would cause a
panic, as the uncomparable type cannot be compared even to itself.

However, one can see that `errors.Cause` has its own loop, and drills
down to the root cause in its own way. There is no need for us to
iterate here.

Instead, we can just take a look at the error itself, and then take a
look at its cause once. If neither is temporary, the error is not
temporary, and we have nothing to worry about.

Signed-off-by: Drew Erny <[email protected]>
@codecov
Copy link

codecov bot commented Aug 24, 2021

Codecov Report

Merging #3024 (39a4233) into master (3cd1651) will increase coverage by 0.09%.
The diff coverage is 60.22%.

@@            Coverage Diff             @@
##           master    #3024      +/-   ##
==========================================
+ Coverage   62.27%   62.37%   +0.09%     
==========================================
  Files         142      155      +13     
  Lines       22888    24531    +1643     
==========================================
+ Hits        14254    15301    +1047     
- Misses       7130     7622     +492     
- Partials     1504     1608     +104     

@dperny dperny merged commit 7956265 into moby:master Aug 30, 2021
dperny added a commit to dperny/docker that referenced this pull request Oct 18, 2021
For details, see github.com/moby/swarmkit/pull/3024.

Vendors a backport of that change.

Signed-off-by: Drew Erny <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant