-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enhance error messages around image to tag resolving. #5920
Enhance error messages around image to tag resolving. #5920
Conversation
We currently surface the plain error returned from the tag resolution without further explanation. This adds wrapping around those errors to explain in which step it fails and adds wrapping around the outer layer to make sure the user gets enough context to be able to tell what exactly failed. Fixes knative#5410
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markusthoemmes: 1 warning.
In response to this:
Fixes #5410
Proposed Changes
We currently surface the plain error returned from the tag resolution without further explanation. This adds wrapping around those errors to explain in which step it fails and adds wrapping around the outer layer to make sure the user gets enough context to be able to tell what exactly failed.
Release Note
Added more expressive error messages around image to tag resolving.
/assign @dgerd
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
} | ||
|
||
func TestResolutionFailed(t *testing.T) { | ||
ctx, cancel, _, controller, _ := newTestController(t) | ||
defer cancel() | ||
|
||
// Unconditionally return this error during resolution. | ||
errorMessage := "I am the expected error message, hear me ROAR!" | ||
controller.Reconciler.(*Reconciler).resolver = &errorResolver{errorMessage} | ||
innerError := errors.New("I am the expected error message, hear me ROAR!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Golint errors: error strings should not be capitalized or end with punctuation or a newline. More info.
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
It's a pity we verify only first possible error return path, though :|
/retest |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, markusthoemmes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
Fixes #5410
Proposed Changes
We currently surface the plain error returned from the tag resolution without further explanation. This adds wrapping around those errors to explain in which step it fails and adds wrapping around the outer layer to make sure the user gets enough context to be able to tell what exactly failed.
Release Note
/assign @dgerd