Skip to content

network: clean up stateless vpack decode error messages#6494

Merged
cce merged 1 commit intoalgorand:masterfrom
cce:clean-vpack-stateless-error-messages
Nov 12, 2025
Merged

network: clean up stateless vpack decode error messages#6494
cce merged 1 commit intoalgorand:masterfrom
cce:clean-vpack-stateless-error-messages

Conversation

@cce
Copy link
Copy Markdown
Contributor

@cce cce commented Nov 12, 2025

Summary

When the stateless vote compression system #6276 reports an error while decoding a message, it can print a scary-looking log message like:

vote decompress error: invalid varuint marker 169 for field \ufffdper: msgpVaruintRemaining: expected fixint or varuint tag, got 0xa9

We can make it a little less scary-looking by removing the msgp marker byte at the beginning so it looks like

vote decompress error: invalid varuint marker 169 for field per: msgpVaruintRemaining: expected fixint or varuint tag, got 0xa9

Test Plan

Tests updated to assert cleaner error strings (they were asserting the ugly ones before)

@cce cce requested a review from Copilot November 12, 2025 14:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error message readability in the stateless vote compression system by removing msgpack field marker bytes from error messages. Instead of displaying raw bytes like \xa3per, error messages now show clean field names like per.

  • Introduced stripMsgpFieldMarker helper function to clean field names in error messages
  • Updated all error message formatting calls to use the new helper
  • Updated test assertions to expect cleaner error strings

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
network/vpack/vpack.go Added stripMsgpFieldMarker function and applied it to all error messages in decoder methods
network/vpack/vpack_test.go Updated test expectations to match cleaner error messages, removed unused fmt import

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 47.42%. Comparing base (3b6cd2d) to head (792f03b).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
network/vpack/vpack.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6494      +/-   ##
==========================================
- Coverage   47.56%   47.42%   -0.15%     
==========================================
  Files         666      659       -7     
  Lines       88478    88397      -81     
==========================================
- Hits        42088    41918     -170     
- Misses      43619    43704      +85     
- Partials     2771     2775       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cce cce merged commit a51dbb4 into algorand:master Nov 12, 2025
41 checks passed
@cce cce deleted the clean-vpack-stateless-error-messages branch November 12, 2025 15:54
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.

4 participants