-
Notifications
You must be signed in to change notification settings - Fork 580
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
chore: display a unique interaction id alongside each error #5737
base: main
Are you sure you want to change the base?
Conversation
623f390
to
3e0391d
Compare
db34eeb
to
eb3adb3
Compare
87142ac
to
8274b3c
Compare
2c06ec0
to
044d048
Compare
044d048
to
e020277
Compare
The PR title says feat, the commit says chore. Let's settle on chore please! And add something meaningful. |
|
7049390
to
8d4289a
Compare
8d4289a
to
355caa7
Compare
00068b1
to
008d6ad
Compare
uiError := userInterface.OutputError(err) | ||
if uiError != nil { | ||
globalLogger.Err(uiError).Msg("ui failed to show error") | ||
} | ||
userInterface.Output(fmt.Sprintf("\nID: %s", interactionId)) |
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.
Question: Showing the a single InteractionId can also be achieved in OutputError()
and would keep the concerns of presenting errors separated from the orchestration logic in main. Is there a reason not to adapt OutputError()
so that it can also work for multiple errors at time point in the future?
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.
After syncing, we decided to wait for design input and then see what is the best way to continue.
Issue: I think the current changes lost a new line, for example when running
Before docs there should be a separator. |
b1b4a3e
to
dcbe9be
Compare
Pull Request Submission Checklist
What does this PR do?
Adds the interaction id to the rendered error when a scan fails
Where should the reviewer start?
review the changes in gaf
How should this be manually tested?
run a test that is expect to fail, eg:
Screenshots
Example of new general CLI failure Error Catalog output:
Example of specific Error Catalog error output: