Open timostamm opened 1 year ago
@matt-lewandowski Is this something that you could consider merging? That would help a lot :) Thanks in advance!
@matt-lewandowski Is this something that you could consider merging? That would help a lot :) Thanks in advance!
Hey @vonsky104 👋
It looks like this PR is outdated and the newer interceptor handles streaming for versions < 0.8.0.
Hey @matt-lewandowski, v0.8.0 is nearly a year old and we've cut a stable v1 since. Would you accept the PR if I update it?
Hey @matt-lewandowski, v0.8.0 is nearly a year old and we've cut a stable v1 since. Would you accept the PR if I update it?
Hey @timostamm 👋
I don't believe we have cut over to v1 just yet. Has there been more breaking changes for V1 or would these changes still just be targeting the v0.8.0 breaking changes? If so, I would be happy to accept the PR
Summary
Update the connect-web-interceptor.js to be compatible with @buildbuild/connect-web >= v0.8.0 for server-streaming RPCs.
Problem description
Release v0.8.0 of @buildbuild/connect-web made a breaking change to interceptors. Unary RPCs are still compatible, but server-streaming RPCs no longer show up in the grpc-web-devtools.
With this change, server-streaming RPCs show up in the grpc-web-devtools again.
Other changes:
connectrpc.eliza.v1.ElizaService/Say
instead of justSay
.emitDefaultValues
for a more human-friendly representation.google.protobuf.Any
is present in the schema.Server-streaming and unary RPCs tested with Chrome and Firefox:
Errors in unary and server-streaming RPCs tested with Chrome and Firefox:
Pros/cons of approach implemented
The behavior for users of versions < 0.8.0 changes with this PR - they will no longer see server-streaming RPCs in the grpc-web-devtools. Since less than 2% of the downloads for of @bufbuild/connect-web are for older versions, this
It would ideal to support type registries for
google.protobuf.Any
, but swallowing seems better than crashing.Checklist
Code Review Guidelines for Reviewers