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

Add Panic Recovery & Logging to Client JSON Unmarshalling #139

Merged
merged 2 commits into from
Mar 28, 2023

Conversation

rtrox
Copy link
Collaborator

@rtrox rtrox commented Mar 28, 2023

Description of the change

This PR adds an unmarshalBody function, which, when the JSON serializer panics, will recover the panic, and return it as an error instead.

If --log-level debug is set, the body of the response will be added to the log message as a string, to allow debugging of the json Decoder.

Benefits

Stability & Debuggability -- JSON failures will not trigger crashes, and when such a failure occurs, --log-level debug will be able to retrieve information to simplify the fix. According to zap's documentation, we should already get a stack trace with error messages of error or higher.

Possible drawbacks

N/A

Applicable issues

rtrox added 2 commits March 28, 2023 08:39

Verified

This commit was signed with the committer’s verified signature. The key has expired.
rtrox Russell Troxel
Signed-off-by: Russell Troxel <russell.troxel@segment.com>

Verified

This commit was signed with the committer’s verified signature. The key has expired.
rtrox Russell Troxel
Signed-off-by: Russell Troxel <russell.troxel@segment.com>
@rtrox rtrox requested a review from onedr0p as a code owner March 28, 2023 18:22
@onedr0p onedr0p merged commit 73fbf8d into master Mar 28, 2023
@onedr0p onedr0p deleted the recover_json branch March 28, 2023 18:33
@onedr0p
Copy link
Owner

onedr0p commented Mar 28, 2023

Nice work @rtrox !

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.

None yet

2 participants