Supports marshaling of nested IEs #18
Conversation
|
@wmnsk |
wmnsk
left a comment
wmnsk
left a comment
There was a problem hiding this comment.
Can you provide the example of payload that this PR will make it possible to handle? 💯 if you can add it to the test case in codec_test.go.
What is the point in returning "Decoded Length is not equal to..." error? In which situation would it happen, and can it be avoided somehow?
Yes, I'll add an example as a test case! But basically it allows you to construct a tree of IEs (IEs containing other IEs) and then marshaling them, without having to manually inject the inner IEs encoded into the outer IEs's Value field.
This error helped me while developing this PR, and I'm sure it could help us catch parsing errors in the wild, and could make potentiel bug reports a bit more readable of what happened in case of issues. Thanks a lot for your time on such a niche but awesome library :-) |
|
Test case added and rebased upon new commit of PR #17, thanks a lot! |
|
Hey @wmnsk, |
|
doh I missed it due to the chaotic post vacation situation 🤦 looks good at a glance, but let me review it again sometime soon this week. Thanks for your patience! |
wmnsk
left a comment
wmnsk
left a comment
There was a problem hiding this comment.
Great work overall, could you check minor comments on the error descriptions?
|
|
||
| c.SetLength() | ||
| if b[1] != c.Length { | ||
| return fmt.Errorf("decoded Length is not equal to Components Length, got %d, expected %d", c.Length, b[1]) |
There was a problem hiding this comment.
I'm not a fan of log-like (redundant) error message - what about this? Or maybe we can even define a custom error like LengthMismatchError with some useful fields, but that's perhaps a bit overkill.
| return fmt.Errorf("decoded Length is not equal to Components Length, got %d, expected %d", c.Length, b[1]) | |
| return fmt.Errorf("component length mismatch, got %d, expected %d", c.Length, b[1]) |
|
|
||
| c.SetLength() | ||
| if expectedLength != c.Length { | ||
| return fmt.Errorf("decoded Length is not equal to Component Length, got %d, expected %d", c.Length, b[1]) |
There was a problem hiding this comment.
Same here.
Also, nicer to have expectedLength in `Components above as well?
| } | ||
| // Dialogue is not a Transaction Payload | ||
| // We have to remove Dialogue from the Payload | ||
| // Or mashaling an unmarshaled message will write back two times the Dialogue |
There was a problem hiding this comment.
| // Or mashaling an unmarshaled message will write back two times the Dialogue | |
| // Or marshaling an unmarshaled message will write back two times the Dialogue |
2 participants
Depends on #17
WIP, unit tests to be fixed.