Skip to content

Conversation

@anthony-treuillier-scality
Copy link
Contributor

No description provided.

@anthony-treuillier-scality anthony-treuillier-scality force-pushed the initial-commit branch 2 times, most recently from bb1c3f2 to 923b484 Compare October 24, 2025 14:44
Comment on lines +194 to +219
func (e *Error) WithDetail(detail string) *Error {
e.Details = append(e.Details, strings.TrimSuffix(detail, "."))

return e
}
Copy link

@eg-ayoub eg-ayoub Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if this is our functional replacement for Wrap, we should add a
WithDetailf function

    func (e *Error) WithDetailf(format string, args ...any) *Error {
        return e.WithDetails(fmt.Sprintf(string, args))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this doesn't replace wrap.
This have to be called on an already created error from the lib, which defy the purpose of wrapping in the first place IMHO

Looking at the previous implementation, the logic difference is clear:

func Wrap(err error, message string) error {
	if err == nil {
		return nil
	}
	err = &withMessage{
		cause: err,
		msg:   message,
	}
	return &withStack{
		err,
		callers(),
	}
}

But I'm all in for @eg-ayoub's suggestion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it from a end purpose point of view,
wrap and wrapf supplement the output of Error()
and with the details object, that purpose is fulfilled, cf. Error() lines 154 to 161

Comment on lines +194 to +219
func (e *Error) WithDetail(detail string) *Error {
e.Details = append(e.Details, strings.TrimSuffix(detail, "."))

return e
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this doesn't replace wrap.
This have to be called on an already created error from the lib, which defy the purpose of wrapping in the first place IMHO

Looking at the previous implementation, the logic difference is clear:

func Wrap(err error, message string) error {
	if err == nil {
		return nil
	}
	err = &withMessage{
		cause: err,
		msg:   message,
	}
	return &withStack{
		err,
		callers(),
	}
}

But I'm all in for @eg-ayoub's suggestion

@anthony-treuillier-scality anthony-treuillier-scality force-pushed the initial-commit branch 4 times, most recently from b8e70d1 to 378e446 Compare October 28, 2025 14:40
Signed-off-by: Anthony TREUILLIER <anthony.treuillier@scality.com>
@TeddyAndrieux TeddyAndrieux deleted the initial-commit branch November 28, 2025 16:44
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.

5 participants