Backwards incompatible change to chunked encoding #495
Comments
|
What marshaller are you using? Does it implement the Delimiter interface? |
|
The default json marshaler. My concern is primarily that the docs say |
|
From looking more in the code it seems that if the json marshaller doesn't implemented the delimited interface it defaults to no delimiter (https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/handler.go#L41) which seems wrong. My expectation would be the default would be a newline (as defined in documentation, and the behavior of previous releases) and if the user wanted to have something else (such as no-delimiter) they'd implement the interface. |
With the addition of the delimiter interface users can now override the delimiter in the stream. With this additional interface we want to maintain compatibility-- namely we want to continue using a newline in the event that the marshaler doesn't implement the new interface (for backwards compatibility). Fixes grpc-ecosystem#495
|
I've submitted a PR which maintains the new interface, but keeps it backwards compatible. |
With the addition of the delimiter interface users can now override the delimiter in the stream. With this additional interface we want to maintain compatibility-- namely we want to continue using a newline in the event that the marshaler doesn't implement the new interface (for backwards compatibility). Fixes #495
With the addition of the delimiter interface users can now override the delimiter in the stream. With this additional interface we want to maintain compatibility-- namely we want to continue using a newline in the event that the marshaler doesn't implement the new interface (for backwards compatibility). Fixes grpc-ecosystem#495
Recently we attempted to update our dependency on grpc-gateway, but after doing so we noticed that streamed responses where no longer formatted the same on the response. Previously each chunk had a newline at the end, so the clients would simply buffer until a newline, then decode a chunk. After digging into the code it seems that this was changed in b0be3cd -- without much explanation as to why.
The entire commit message is
runtime: fix chunk encoding-- which is confusing since it doesn't reference an issue or anything else. At this point this breaks all my clients (meaning I have to remain on an older version) and I'm not even sure this is a change we'd want (as it makes client implementations a bit more of a pain).I attempted to find an issue explaining the reasoning, but was unsuccessful. IMO that commit should be reverted (although I'd keep the test-- just change it). But if there was a reason to make this change I'd like to understand why.
The text was updated successfully, but these errors were encountered: