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

help diagnostics by reporting the position, step and end of the data being decoded #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GerHobbelt
Copy link
Contributor

@GerHobbelt GerHobbelt commented Jan 4, 2022

help diagnostics by reporting the position, step and end of the data being decoded when the decoder throws an exception: make those info bits part of the exception message so we can find out where the shit hit the fan for much faster problem diagnosis.

This is a fast & shoddy hack done as part of digging up why run_tests.sh went pear-shaped on MS Windows dev box (#170); the code is merely to showcase the intended functionality: the existing exception was equivalent to ye ole "syntax error" re information content 😉 , which made debugging hard. When I introduced the mention of these offsets into binary input data, I was able to spot the issue immediately.

The offsets are part hex, part decimal, so they match nicely with od -t x1 dump output I had available. Just a bit of counting and ah-ha! 0d 0a. Suspect! 🥳


AFAIAC this is more about the idea (making the error reports more informative) than a code submission per se: I know some folks will reel from the (nasty) mixed C/C++ idiom used here in the augmented exception class.

…being decoded when the decoder throws an exception: make those info bits part of the exception message so we can find out *where* the shit hit the fan for much faster problem diagnosis.

# Conflicts:
#	Core/Meta/VersionInfo.cs
@mattiekat
Copy link
Contributor

@GerHobbelt I was just looking though things and noticed this PR sitting. Did you want us to review?

@mattiekat mattiekat requested a review from lynn January 31, 2022 22:30
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.

2 participants