Merged
Conversation
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Contributor
There was a problem hiding this comment.
Hey @janisz - I've reviewed your changes - here's some feedback:
- Extract the duplicate Wrapf logic in Unmarshal into a shared fallback helper (similar to Marshal) to reduce code duplication and ensure consistency.
- Standardize the error message templates and placeholder usage across both Marshal and Unmarshal wraps so failure logs are uniform and easy to search.
- Note that when the fallback succeeds, the original codec error is dropped—consider preserving or logging that initial error so the root cause isn’t lost.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
9 tasks
Contributor
Author
|
/retest |
Contributor
|
Images are ready for the commit at 65a4828. To use with deploy scripts, first |
Contributor
|
Tomek, show me this in action with some examples what it does, cause the expectations suggest that it may help a lot with debugging grpc problems as the few recent ones that we are handling. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
=======================================
Coverage 49.12% 49.13%
=======================================
Files 2563 2563
Lines 188228 188232 +4
=======================================
+ Hits 92476 92480 +4
Misses 88463 88463
Partials 7289 7289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 similar comments
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
=======================================
Coverage 49.12% 49.13%
=======================================
Files 2563 2563
Lines 188228 188232 +4
=======================================
+ Hits 92476 92480 +4
Misses 88463 88463
Partials 7289 7289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
=======================================
Coverage 49.12% 49.13%
=======================================
Files 2563 2563
Lines 188228 188232 +4
=======================================
+ Hits 92476 92480 +4
Misses 88463 88463
Partials 7289 7289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
=======================================
Coverage 49.12% 49.13%
=======================================
Files 2563 2563
Lines 188228 188232 +4
=======================================
+ Hits 92476 92480 +4
Misses 88463 88463
Partials 7289 7289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
=======================================
Coverage 49.12% 49.13%
=======================================
Files 2563 2563
Lines 188228 188232 +4
=======================================
+ Hits 92476 92480 +4
Misses 88463 88463
Partials 7289 7289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15283 +/- ##
==========================================
+ Coverage 49.12% 49.23% +0.10%
==========================================
Files 2563 2573 +10
Lines 188228 188858 +630
==========================================
+ Hits 92476 92992 +516
- Misses 88463 88546 +83
- Partials 7289 7320 +31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
I have just tested that for you by using the image from this PR on Central and sending the garbled message using local-sensor. Central has panicked: |
rhybrillou
reviewed
May 15, 2025
Contributor
I thought, I pasted entire log starting from a moment where everything was healthy until the end of the log. |
Contributor
Author
|
/retest |
rhybrillou
approved these changes
May 21, 2025
Co-authored-by: Yann Brillouet <91869377+rhybrillou@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR wrap errors that occured in grpc codec to get more information what failed exactly.
Testing
Automated testing
How I validated my change
CI