-
Notifications
You must be signed in to change notification settings - Fork 14
Description
The unary response handler writes grpc-status and grpc-message trailers as bytestrings, but sonora.protocol.pack_trailers only handles decoded str types, leading to the repr for the bytestrings being serialized, resulting in the trailers being actually the string "b'grpc-status'" and "b'grpc-message'" (with the literal b) rather than "grpc-status" and "grpc-mesage"
The streaming response and WSGI handlers are correct in that they send strs to pack_trailers, which get serialized fine. This is an issue especially for clients that assert a status trailer (or header) is present, and there is a key-miss on the trailer having extra b and ' characters in it
It seems like to be consistent with the rest of the repo, trailers should always be str, although maybe confusion arose from headers needing to be pairs of bytes rather than str per the ASGI spec, and the _do_grpc_error handler always writes statuses out as headers rather than trailers, so there are multiple ways headers (and trailing headers/trailers) are written out. An easy fix would be to just change the unary response trailers from bytes to str, especially since unpack_trailers deserializes to str, but maybe handling all trailers as bytes to be consistent with the ASGI spec on headers (and trailers) would be a more consistent approach with upstream standards, or just support bytes and str in pack_trailers
I think this probably wasn't a detected issue sooner because clients might not assert ok statuses being present in healthy responses, but we ran into the issue when connect-es started asserting status trailers in v1.5.0