Conversation
|
|
||
| $headers = ['Content-Type' => 'application/x-protobuf'] + $headers; | ||
| $headers += ['x-goog-api-client' => []]; | ||
| $headers['x-goog-api-client'][] = 'grpc-web'; |
There was a problem hiding this comment.
There seems to be a bug here:
We are using:
$headers += ['x-goog-api-client'] => []];That is great as we are trying to not overwrite something that already exists, so far so good. But if we check for example in this test we are passing the x-goog-api-client as a string:
$client = new ResumableUploadClient(
$this->createStubTransport($requestBuilder->reveal(), $httpHandler),
$this->prophesize(CredentialsWrapper::class)->reveal(),
headers: ['x-goog-api-client' => 'test-agent/1.0']
);So the $headers += ['x-goog-api-client'] => []]; line tries to assign an array but it does not overwrites it. But the header is still a string, and the next line:
$headers['x-goog-api-client'][] = 'grpc-web';We try to add grpc-web to an array, but it is a string, turning into an invalid string operatior:
`some_string`[] = 'grpc-web';
There was a problem hiding this comment.
Good catch... and also, the HttpUnaryTransporTrait isn't grpc-web, nor should it have a content type of application/x-protobuf... That should be just in GrpcFallbackTransport, which doesn't need to support ResumableUploads anyway.
I think we should just remove the ResumableUploadTransportInterface from GrpcFallbackTransport, and move these methods to RestTransport. That will simplify things. There is no need to support ResumableUploads on GrpcFallbackTransport at this time (I did it mostly just for fun and to see what the design would look like, but it's over-complicated for the initial release)
There was a problem hiding this comment.
Ok I've moved these methods to RestTransport, revert the adding the interface to GrpcFallbackTransport and reverted all changes to HttpUnaryTransportTrait, and added tests in RestTransport specifically for the new methods.
No description provided.