-
Notifications
You must be signed in to change notification settings - Fork 483
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
Fix scenarios where error messages aren't propagated correctly (#424) #425
Conversation
* Improve tests for agent and service/remote to expose bad behaviors * Breakdown test into multiple ones to make them more clear * Fix scenarios where error messages aren't propagated correctly --------- Co-authored-by: Felippe Durán <felippe.duran@wildlifestudios.com>
When running tests we need to ensure a graceful cleanup by terminating/closing the context by calling the Close() method, which should call the cancelFunc of the group service context. This should graceful handle goroutines like groupTTLCleanup with a sane context cancellation
c4fe5e7
to
da69a18
Compare
Pull Request Test Coverage Report for Build 12353837806Details
💛 - Coveralls |
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.
Will GetErrorFromPayload keep working if the server is using json serializer?
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.
Yes. The difference between unmarshalling to e.Error
struct and protos.Error
is a few internal XXX fields:
// Error is an error with a code, message and metadata
type Error struct {
Code string
Message string
Metadata map[string]string
}
type Error struct {
Code string `protobuf:"bytes,1,opt,name=code" json:"code,omitempty"`
Msg string `protobuf:"bytes,2,opt,name=msg" json:"msg,omitempty"`
Metadata map[string]string `protobuf:"bytes,3,rep,name=metadata" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"`
XXX_NoUnkeyedLiteral struct{} `json:"-"`
XXX_unrecognized []byte `json:"-"`
XXX_sizecache int32 `json:"-"`
}
So JSON serializer should still be able to serialize to protos.Error struct, just the XXX fields will be empty
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.
only one question. apart from it, LGTM
Porting the routing fix that @felippeduran provided to main/v3. See #424