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

Print a more meaningful message when decoding an object fails. #917

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

partim
Copy link
Member

@partim partim commented Jan 10, 2024

This PR changes the error message printed when decoding any of the RPKI objects fails from whatever wondrous thing the decoder says to a generic message stating that decoding an object of a certain expected type failed.

@partim partim linked an issue Jan 10, 2024 that may be closed by this pull request
@partim partim requested a review from a team January 10, 2024 11:31
Copy link
Member

@ximon18 ximon18 left a comment

Choose a reason for hiding this comment

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

The change itself is technically fine, and I can see that this is an improvement for operators, but it could also add the improved error context without discarding the actual underlying error message. Is it really wise to discard the underlying error message or is it the case that it's okay to discard it because the underlying message is not and will never be of any actual use in daignosing the cause of the probelm?

@partim
Copy link
Member Author

partim commented Jan 15, 2024

The decoder does indeed not provide any useful information. This is a limitation of the current error handling model in both bcder and rpki-rs. It can (and should) be fixed at some point. This PR really only provides a work-around for showing missing further values (at position 2) to users as the only error reason.

@partim partim merged commit 7783587 into series-0.13 Jan 16, 2024
10 checks passed
@partim partim deleted the 0.13-meaningfull-parse-error branch January 16, 2024 09:05
partim added a commit that referenced this pull request Jan 17, 2024
New

* Added support for private keys marked as “EC PRIVATE KEY“ in the PEM files
  for TLS server configuration. ([#921])
* The rsync collector now logs stderr output of the rsync command directly
  instead of collecting it and logging it in one go after the commend
  returned. ([#290])

Bug Fixes

* The `dump` command will now succeed even if certain directories or files
  in the repository cache are missing. ([#916])
* A more meaningful message is now printed when decoding RPKI objects
  fails. It will still not give much detail but at least it isn’t
  confusing any more. ([#917])

Other changes

* Updated the `nlnetlabs-testbed` TAL to the current location and key.
  ([#922])
partim added a commit that referenced this pull request Jan 24, 2024
New

* Added support for private keys marked as “EC PRIVATE KEY“ in the PEM files
  for TLS server configuration. ([#921])
* The rsync collector now logs stderr output of the rsync command directly
  instead of collecting it and logging it in one go after the commend
  returned. ([#290])

Bug Fixes

* The `dump` command will now succeed even if certain directories or files
  in the repository cache are missing. ([#916])
* A more meaningful message is now printed when decoding RPKI objects
  fails. It will still not give much detail but at least it isn’t
  confusing any more. ([#917])

Other changes

* Updated the `nlnetlabs-testbed` TAL to the current location and key.
  ([#922])
partim added a commit that referenced this pull request Jan 24, 2024
This PR changes the error message printed when decoding any of the RPKI
objects fails from whatever wondrous thing the decoder says to a generic
message stating that decoding an object of a certain expected type failed.

This PR is a port of #917, originally added to the 0.13 series.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 25, 2024
Pkgsrc changes:
 * Bump version & re-compute cargo-depends.

Upstream changes:

New

* Added support for private keys marked as "EC PRIVATE KEY" in the
  PEM files for TLS server configuration. ([#921])
* The rsync collector now logs stderr output of the rsync command
  directly instead of collecting it and logging it in one go after
  the commend returned. ([#290])

Bug Fixes

* The `dump` command will now succeed even if certain directories
  or files in the repository cache are missing. ([#916])
* A more meaningful message is now printed when decoding RPKI
  objects fails. It will still not give much detail but at least it
  isn't confusing any more. ([#917])

Other changes

* Updated the `nlnetlabs-testbed` TAL to the current location and
  key. ([#922])

[#916]: NLnetLabs/routinator#916
[#917]: NLnetLabs/routinator#917
[#920]: NLnetLabs/routinator#920
[#921]: NLnetLabs/routinator#921
[#922]: NLnetLabs/routinator#922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

print human error message for ASPA pre-16
2 participants