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
Adding a better error message for download mode #1492
base: main
Are you sure you want to change the base?
Changes from all commits
0224e53
fa2631c
834eba4
f45a03b
4715103
e928a0f
32e7c28
a846557
25b5860
36b6d62
2c6b90b
5b85142
65662f5
e962a4d
1b29dcf
d6a7cbd
de71cb7
cc70e12
a5eda16
198d574
ee8f4d1
2cc9f84
9695c36
726b77c
4320101
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package errors | ||
|
||
import ( | ||
"fmt" | ||
"strings" | ||
) | ||
|
||
// Config returns an error specifically tailored for reporting errors with configuration | ||
// values. You can check for these errors by calling errors.Is(err, KindConfigError) | ||
// (from the github.com/gomods/athens/pkg/errors package). | ||
// | ||
// Generally these kinds of errors should make Athens crash because it was configured | ||
// improperly | ||
func Config(op Op, field, helpText, url string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this ever be called from outside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. I put it here just because I think it makes more sense to group a configuration error under the |
||
slc := []string{ | ||
fmt.Sprintf("There was a configuration error with %s. %s", field, helpText), | ||
} | ||
if url != "" { | ||
slc = append(slc, fmt.Sprintf("Please see %s for more information.", url)) | ||
} | ||
return E(op, KindConfigError, strings.Join(slc, "\n\t")) | ||
} |
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.
I think this error is now less readable, because before it used to say
download mode is not set
but now it will sayisn't a valid value
(notice the space in the beginning because there's nothing)I think in the Sprintf of the validate function we should pass a
%q
so it would say"" isn't a valid value
or we should keep the old error here for better messaging.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.
How about if we pass
%q
as you said, and also prefix the error with "Download mode ..."?The result would then be
Download mode "" isn't a valid value
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.
That sounds great