Skip to content

Conversation

lukedirtwalker
Copy link
Collaborator

@lukedirtwalker lukedirtwalker commented Nov 12, 2019

Also log verification error in all seghandler using code.
Also clean up error and make sure metrics report this correctly.

Fixes #3357
Fixes #3351


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 10 of 10 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)


go/lib/infra/modules/seghandler/seghandler.go, line 116 at r1 (raw file):

	result.verifyErrs = append(allVerifyErrs, verifyErrs...)
	result.stats.verificationErrs(result.verifyErrs)
	if err == nil && result.stats.SegVerifyErrors() == units {

is equivalent to:

if err != nil {
  result.err = serrors.Wrap(ErrDB, err)
} else if result.stats.SegVerifyErrors() == units {
  result.err = serrors.Wrap(ErrVerification, result.verifyErrs.ToError())
}

or

switch {
case err != nil:
  result.err = serrors.Wrap(ErrDB, err)
case result.stats.SegVerifyErrors() == units:
  result.err = serrors.Wrap(ErrVerification, result.verifyErrs.ToError())
}

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.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

Also log verification error in all seghandler using code.
Also clean up error and make sure metrics report this correctly.

Fixes scionproto#3357
Fixes scionproto#3351
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 1 of 1 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

@lukedirtwalker lukedirtwalker merged commit 366f3c7 into scionproto:master Nov 13, 2019
@lukedirtwalker lukedirtwalker deleted the pubsciondfetchcert branch November 13, 2019 12:15
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.

segfetcher: Error out if verification of all segments in a reply fails. SD,PS: Log verification issues.
2 participants