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

proposal: errors: add errors.Mark to support error marks #50819

Closed
searKing opened this issue Jan 26, 2022 · 13 comments
Closed

proposal: errors: add errors.Mark to support error marks #50819

searKing opened this issue Jan 26, 2022 · 13 comments

Comments

@searKing
Copy link

searKing commented Jan 26, 2022

Issue #50800 proposes a way to support error marks without any side effect.

Background

Currently, fmt.Errorf("error occur: %w", err) supports traversing a linked chain of errors by %w, but parallel %w is not allowed, such as fmt.Errorf("error occur: %w, %w", err) .

Concerns

We need one general way to support marks, that is:

  • Non-Invasive for add error marks
  • Without any side effect
  • Support both linked chain and parallel marks
  • errors.Is returns true if and only if err or any marks match the target err
  • Omit marks if error is nil
  • Marks take effects only when errors.Is and '%v' in fmt.

Proposed changes to the errors package

I propose that the following API be added to the errors package:

func Mark(err error, marks ...error) error

Ideally, a developer could write the following error handling:

package main

import (
	"errors"
	"fmt"
	"io/fs"
	"os"
)

func main() {
	var markOneErr = errors.New("mark-one")
	var markTwoErr = errors.New("mark-two")
	_, err := os.Open("non-existing")

	err = errors.Mark(err, markOneErr, markTwoErr)

	if err != nil {
		fmt.Println("actual error: ", err)

		if errors.Is(err, fs.ErrNotExist) {
			fmt.Println("file does not exist")
		}
		if errors.Is(err, markOneErr) {
			fmt.Println("a mark-one err occur: ", err)
		}
		if errors.Is(err, markTwoErr) {
			fmt.Println("a mark-two err occur: ", err)
		}
		if !errors.Is(err, fs.ErrNotExist) && !errors.Is(err, markOneErr) && !errors.Is(err, markTwoErr)  {
			fmt.Println(err)
		}
	}

	// Output:
	// actual error: file does not exist
	// file does not exist
	// a mark-one err occur:  open non-existing: no such file or directory
	// a mark-one err occur:  open non-existing: no such file or directory
}

Workarounds

The following is a proposed implementation of errors.Mark in Issue #50800 :

var _ error = markError{} // verify that Error implements error

// Mark returns an error with the supplied errors as marks.
// If err is nil, return nil.
// marks take effects only when Is and '%v' in fmt.
// Is returns true if err or any marks match the target.
func Mark(err error, marks ...error) error {
	if err == nil {
		return nil
	}
	if len(marks) == 0 {
		return err
	}
	me := markError{
		err:   err,
		marks: marks,
	}
	return me
}

type markError struct {
	err   error   // visual error
	marks []error // hidden errors as marks, take effects only when Is and '%v' in fmt.
}

func (e markError) Error() string {
	if e.err == nil {
		return ""
	}
	return e.err.Error()
}

func (e markError) Format(s fmt.State, verb rune) {
	if e.err == nil {
		return
	}
	switch verb {
	case 'v':
		if s.Flag('+') {
			me := e.clean()
			if len(me.marks) == 0 {
				_, _ = fmt.Fprintf(s, "%+v", me.err)
				return
			}
			_, _ = io.WriteString(s, "Marked errors occurred:\n")

			_, _ = fmt.Fprintf(s, "|\t%+v", me.err)
			for _, mark := range me.marks {
				_, _ = fmt.Fprintf(s, "\nM\t%+v", mark)
			}
			return
		}
		fallthrough
	case 's', 'q':
		_, _ = io.WriteString(s, e.Error())
	}
}

// clean removes all none nil elem in all the marks
func (e markError) clean() markError {
	var marks []error
	for _, err := range e.marks {
		if err != nil {
			marks = append(marks, err)
		}
	}
	return markError{
		err:   e.err,
		marks: marks,
	}
}

// Is reports whether any error in markError or it's mark errors matches target.
func (e markError) Is(target error) bool {
	if Is(e.err, target) {
		return true
	}
	for _, err := range e.marks {
		if Is(err, target) {
			return true
		}
	}
	return false
}

// Unwrap returns the error in e, if there is exactly one. If there is more than one
// error, Unwrap returns nil, since there is no way to determine which should be
// returned.
func (e markError) Unwrap() error {
	return e.err
}

You can also find an implementation with test cases in the Go Playground: https://go.dev/play/p/J6-Zn4QBC81

See also

proposal: errors: simplified error inspection

@gopherbot gopherbot added this to the Proposal milestone Jan 26, 2022
@ianlancetaylor
Copy link
Member

It appears that this can be implemented in a third party package. Why should this be in the standard library? See https://go.dev/doc/faq#x_in_std.

@ianlancetaylor
Copy link
Member

What is an error mark? I'm not familiar with the term. Thanks.

@seankhliao
Copy link
Member

how is this different from the previously declined multi error proposals, eg #47811

@searKing
Copy link
Author

how is this different from the previously declined multi error proposals, eg #47811

  • error marks only affect errors.Is for Equal and '%v' in fmt for details, it's hidden otherwise
  • multi error affect all functions in package errors and fmt, it behaves like a new error actually

In my case, we need errors.Mark if we just want to mark current to handle specially later, like same error with different code

func readImage() error {
	_, err := os.Open("non-existing-image")
	return errors_.Mark(err, ErrReadImageFile)
}

func readVideo() error {
	_, err := os.Open("non-existing-video")
	return errors_.Mark(err, ErrReadVideoFile)
}

type ErrorCode int

const (
	ErrorCodeReadImageFile ErrorCode = 1
	ErrorCodeReadVideoFile ErrorCode = 2
)

func PrintError(err error) {
	if errors.Is(err, ErrReadImageFile) {
		fmt.Printf("code(%d): msg:%s", ErrorCodeReadImageFile, err.Error())
		return
	}
	if errors.Is(err, ErrReadVideoFile) {
		fmt.Printf("code(%d): msg:%s", ErrorCodeReadVideoFile, err.Error())
		return
	}
}
func main() {
	PrintError(readImage())
	PrintError(readVideo())
}

@searKing
Copy link
Author

searKing commented Jan 26, 2022

What is an error mark? I'm not familiar with the term. Thanks.

  • May be error mark is not accurate. It's like a syntax sugar to wrap the original error with custom types.

  • The purpose of error mark is to add additional context to an error. Additional context take effects if and only if used for error type comparison.

  • Without the errors.Mark, I need to add additional context to an error by implementing a custom type

type QueryError struct {
    Err   error
}
func (e *QueryError) Unwrap() error { return e.Err }
func (e *QueryError) Error() string { return e.Err.Error() }
...
err := QueryError{ Err: fmt.Errorf("access denied: %w", ErrPermission)) }
...
var e *QueryError
// Note: *QueryError is the type of the error.
if errors.As(err, &e) && errors.Is(err, ErrPermission) {
    // query failed because of a permission problem
}
  • Using the errors.Mark function, we can write this as:
err := errors.Mark(fmt.Errorf("access denied: %w", ErrPermission), ErrQuery)
...
if errors.Is(err, ErrQuery) && errors.Is(err, ErrPermission)...

@rsc
Copy link
Contributor

rsc commented Jan 26, 2022

I am confused about the relation between this issue and #50800. Usually we use the same issue for the proposal and the actual implementation (if the proposal is accepted). Should #50800 be closed as a duplicate of this one?

@seankhliao
Copy link
Member

#50800 is a PR / CL / implementation which is on hold while this issue / proposal goes through the process

@ianlancetaylor
Copy link
Member

Thanks for the replies. I still don't see a reason why this should be in the standard library.

@searKing
Copy link
Author

Thanks for the replies. I still don't see a reason why this should be in the standard library.

  • Just as easy to construct errors with hidden marks, as the reason of why %w was introduced to fmt.Errorf:We believe that it is important for constructing unwrappable errors to be just as easy, without forcing users to learn a new, different API. The %w verb is a minimal change that does exactly that. We will keep support for the %w format verb in Go 1.13.
  • I think a generic and graceful solution or syntax sugar could be added to the standard library if and only if a consensus arises.

Whether or not this proposal is accepted, I'm curious how others have approached this problem in a graceful and go-native way.

@rsc
Copy link
Contributor

rsc commented Feb 2, 2022

Sorry, missed that #50800 was a PR and not an issue.

As Ian said, it seems like it would make sense to implement this outside the standard library and gauge how useful it is.

@rsc
Copy link
Contributor

rsc commented Feb 2, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 9, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants