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

errors: Use common.ErrMsg for constants #3314

Merged
merged 3 commits into from
Nov 5, 2019

Conversation

lukedirtwalker
Copy link
Collaborator

@lukedirtwalker lukedirtwalker commented Nov 5, 2019

This helps to move to a more friendly go 1.13 error environment:

  1. the error constants can be detected with xerrors.Is.
  2. since common.ErrMsg is also an error we can easily migrate common.NewBasicError calls towards the corresponding serrors calls.
  3. in the future we can replace all common.ErrMsg in one go with serrors.New.

This change is Reviewable

Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewed 89 of 89 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @lukedirtwalker)


go/cert_srv/setup.go, line 43 at r1 (raw file):

const (
	ErrorConf      common.ErrMsg = "Unable to load configuration"

Err


go/cert_srv/internal/config/config.go, line 43 at r1 (raw file):

	ReissueReqTimeout = 5 * time.Second

	ErrorKeyConf   common.ErrMsg = "Unable to load KeyConf"

Err


go/lib/keyconf/keyconf.go, line 72 at r1 (raw file):

// Errors
const (
	ErrorOpen    common.ErrMsg = "Unable to load key"

ErrOpen same below


go/lib/scrypto/validity.go, line 33 at r1 (raw file):

	ErrNotBeforeNotSet = errors.New("not_before not set")
	// ErrErrInvalidValidityPeriod indicates an invalid validity period.
	ErrErrInvalidValidityPeriod = errors.New("not_after before not_before")

ErrErr?


go/lib/scrypto/version.go, line 28 at r1 (raw file):

// ErrErrInvalidVersion indicates an invalid trust file version.
var ErrErrInvalidVersion = errors.New("version must not be zero")

ErrErr?


go/lib/scrypto/cert/chain.go, line 242 at r1 (raw file):

	}
	if err = validateFields(m, chainFields); err != nil {
		return common.NewBasicError(ErrUnableValidateFields, err)

ErrValidatingFields


go/lib/scrypto/trc/entries.go, line 49 at r1 (raw file):

	}
	if err = validateFields(m, coreASFields); err != nil {
		return common.NewBasicError(ErrUnableValidateFields, err)

ErrValidatingFields


go/lib/scrypto/trc/trc.go, line 42 at r1 (raw file):

// Error strings
const (
	EarlyUsage          common.ErrMsg = "Creation time in the future"

Err all the things


go/lib/spkt/cmnhdr.go, line 42 at r1 (raw file):

const (
	ErrorUnsuppVersion common.ErrMsg = "Unsupported SCION version"

Err


go/lib/topology/raw.go, line 32 at r1 (raw file):

const (
	ErrorOpen    common.ErrMsg = "Unable to open topology"

Err

Copy link
Collaborator Author

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: 74 of 88 files reviewed, 10 unresolved discussions (waiting on @oncilla)


go/cert_srv/setup.go, line 43 at r1 (raw file):

Previously, Oncilla wrote…

Err

Done.


go/cert_srv/internal/config/config.go, line 43 at r1 (raw file):

Previously, Oncilla wrote…

Err

Done.


go/lib/keyconf/keyconf.go, line 72 at r1 (raw file):

Previously, Oncilla wrote…

ErrOpen same below

Done.


go/lib/scrypto/validity.go, line 33 at r1 (raw file):

Previously, Oncilla wrote…

ErrErr?

Done.


go/lib/scrypto/version.go, line 28 at r1 (raw file):

Previously, Oncilla wrote…

ErrErr?

Done.


go/lib/scrypto/cert/chain.go, line 242 at r1 (raw file):

Previously, Oncilla wrote…

ErrValidatingFields

Done.


go/lib/scrypto/trc/entries.go, line 49 at r1 (raw file):

Previously, Oncilla wrote…

ErrValidatingFields

Done.


go/lib/scrypto/trc/trc.go, line 42 at r1 (raw file):

Previously, Oncilla wrote…

Err all the things

Done.


go/lib/spkt/cmnhdr.go, line 42 at r1 (raw file):

Previously, Oncilla wrote…

Err

Done.


go/lib/topology/raw.go, line 32 at r1 (raw file):

Previously, Oncilla wrote…

Err

Done.

Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewed 16 of 16 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

This PR prepares for migration from common.NewBasicError towards serrors.
@lukedirtwalker lukedirtwalker merged commit 5cd9f8b into scionproto:master Nov 5, 2019
@lukedirtwalker lukedirtwalker added the refactor Change that focuses around reducing tech debt label Nov 5, 2019
@lukedirtwalker lukedirtwalker deleted the pubErrMigrate branch November 5, 2019 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Change that focuses around reducing tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants