-
Notifications
You must be signed in to change notification settings - Fork 81
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
add details about underlying message types on input message protocol errors #1252
Conversation
…errors Signed-off-by: Aman Karmani <aman@tmm1.net>
7737461
to
80f838c
Compare
Hey @tmm1 Can you tell us a bit about how often you encountered protocol errors and what the backend was? Because we don't expect users to ever hit these errors. |
We are seeing large numbers of these proto errors and are trying to figure out why. Adding these details here would help. |
{ | ||
methodName: spec.method.name, | ||
serviceTypeName: spec.service.typeName, | ||
inputTypeName: spec.method.I.typeName, | ||
outputTypeName: spec.method.O.typeName, | ||
url: context.url, | ||
requestMethod: context.requestMethod, | ||
} |
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.
Metadata is used for errors returned as part of the protocol, fully qualified method name should be sufficient to pin point the rpc
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.
If I understand correctly, you are suggesting I remove this as adding metadata would interfere with other parts of the stack?
I can do that. I agree that the rpc method is the main piece of information missing.
@@ -149,8 +149,16 @@ export function transformInvokeImplementation< | |||
const input2 = await inputIt.next(); | |||
if (input2.done !== true) { | |||
throw new ConnectError( | |||
"protocol error: received extra input message for unary method", | |||
`protocol error: received extra input message ${spec.method.I.typeName} for unary method ${spec.method.name}`, |
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.
Fully qualified method name is unique and will be sufficient to pinpoint:
`protocol error: received extra input message ${spec.method.I.typeName} for unary method ${spec.method.name}`, | |
`protocol error: received extra input message for unary method ${spec.service.typeName}.${spec.method.name}`, |
Hey @tmm1 Can you tell us what is the client in use? |
we're using connect-es v1.4.0 |
Thanks for the guidance. I believe this may be an application specific issue, so I will close for now. |
in general the protocol errors are all missing a lot of context