-
-
Notifications
You must be signed in to change notification settings - Fork 522
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
Handle receiving empty member assignment #567
Conversation
…fkajs into handle-empty-member-assignment
types/index.d.ts
Outdated
@@ -338,7 +338,7 @@ export type MemberAssignment = { | |||
|
|||
export const AssignerProtocol: { | |||
MemberMetadata: ISerializer<MemberMetadata> | |||
MemberAssignment: ISerializer<MemberAssignment> | |||
MemberAssignment: ISerializer<MemberAssignment | null> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means MemberAssignment#encode() would need to handle null
as well now, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! Took me a bit to see what you meant, but yes indeed, it would mean that encode
would also need to handle receiving null.
I changed it now so that only the decode
has a nullable return type.
@Nevon can you add an integration test? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
The test assigner only returns assignments for the first member of the group, and not the second one. Without #567 the second consumer will crash when trying to decode the missing assignment.
Fixes #550.
This is a slightly different approach than #551 because I wanted to make sure that the MemberAssignment decoder could handle receiving a buffer with insufficient data, instead of expecting the caller to do the checking for it.