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

Doc: Clarify distinction between WithDetail() and WithHint() #114

Closed
omarkohl opened this issue Jul 31, 2023 · 2 comments
Closed

Doc: Clarify distinction between WithDetail() and WithHint() #114

omarkohl opened this issue Jul 31, 2023 · 2 comments

Comments

@omarkohl
Copy link
Contributor

By reading the source code and the existing documentation (e.g. README.md or code docstrings) I understand that WithDetail() and WithHint() can be used in the same way.

README states for both:

when to use: need to embark a message string to output when the error is presented to a human.

The source code hintdetail/with_detail.go and hintdetail/with_hint.go is also essentially identical.

So what is the difference? Maybe this library is trying to be non-prescriptive and giving its users freedom to use these functions and types as it suits them, but it is giving two mechanisms for apparently solving the same problem. Giving some guidance how this was originally intended is probably valuable to make usage more uniform and thereby make understanding and contributing to other people's code easier.

If I check the CockroachDB source code for instances of these functions I get the impression that WithDetail() is meant more for internal use to augment errors with additional information for debugging whereas WithHint() is meant for giving end users hints about what is wrong with the data they provided. But I might be misunderstanding or cherry-picking examples.

e.g.

err := errors.WithDetail(errors.Newf(
	"attempting to discard the zone configuration of a multi-region entity"),
	"discarding a multi-region zone configuration may result in sub-optimal performance or behavior",
)
return errors.WithDetail(
	pgerror.WithConstraintName(
		pgerror.Newf(
			pgcode.UniqueViolation, "%s %q", errMsg, constraintName,
		),
		constraintName,
	),
	fmt.Sprintf(
		"Key (%s)=(%s) is duplicated.", strings.Join(colNames, ","), strings.Join(valuesStr, ","),
	),
)
if err != nil {
	topLevelError(errors.WithDetail(
		errors.Wrapf(err, "parsing statement %d", i+1), s.SQL),
		http.StatusBadRequest)
	return
}
err = errors.WithHint(err, "Try changing the CWD of the cockroach process to a writable directory.")
if err != nil {
	return errors.WithHint(
		errors.Wrapf(err, "job %d could not be loaded", jobs[i]),
		"The job may not have succeeded.")
}

How to improve

What I suggest is making some distinction in the README "Available wrapper constructors" section and in the WithDetail() and WithHint() docstring.

@knz
Copy link
Contributor

knz commented Jul 31, 2023

Your interpretation is right.

Moreover, in the flatten functions details get concatenated but hints get deduplicated.

knz added a commit that referenced this issue Aug 10, 2023
Document intended differences between WithHint and WithDetail (#114)
@knz
Copy link
Contributor

knz commented Aug 10, 2023

addressed in #116.

@knz knz closed this as completed Aug 10, 2023
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

No branches or pull requests

2 participants