grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2.01k stars 414 forks source link

Testing using `FakeChannel` with enqueueing error and trailing metadata, but trailing metadata is never received. #1258

Open IrisG33 opened 3 years ago

IrisG33 commented 3 years ago

I try to write unit tests on handling received trailing metadata and error from gRPC.

My code that handling the responses:

var trailingMetadata = [String: String]()
unaryRPC.trailingMetadata.whenSuccess { result in trailingMetadata = result.headers }

let status = try! unaryRPC.status.wait()
if !status.isOk {
   print(status)
   print(trailingMetadata)
}

My test setup:

let fakeTrailingMetadata = HPACKHeaders([("fake1", "fake2")])
let fakeError = FakeError()

let stream: FakeUnaryResponse<FakeRequest, FakeResponse> = self.fakeChannel
      .makeFakeUnaryResponse(path: requestPath, requestHandler: requestHandler)

/// With this, the `unaryRPC.trailingMetadata.whenSuccess` was never invoked. `unaryRPC.trailingMetadata.whenComplete`
/// is invoked, but only get `fakeError` from result, can't retrieve the fake trailing metadata.
try! stream.sendError(fakeError, trailingMetadata: fakeTrailingMetadata)

I wonder is there something wrong with my test setup? Also in the real implementation, if error is returned, will I still receive trailing metadata if provided by server?

glbrntt commented 3 years ago

This is a bug, I think. When we introduced interceptors a while back some of the grpc internals were changed so that trailers get delivered along with the status of the RPC, so the trailers get held until we see the status. Here we're sending trailers and internally we're holding on to them until we receive a status, instead we receive an error, so the trailers never get sent on to the caller.


Also in the real implementation, if error is returned, will I still receive trailing metadata if provided by server?

Usually not: the status is derived from the trailers received from the server and is the final part we receive on the RPC. So if we hit an error then chances are it's before we received the trailers.

eseay commented 2 years ago

Hey @glbrntt - do you have any update on this or might you be able to provide me some direction so that I could attempt a fix and PR myself?

The systems I am interacting with are storing structured error model data in grpc-status-details-bin, so in order to test my RPCs' failure cases, I rely upon being able to read trailingMetadata under test.

I am using the v1.7.3 release but am awaiting trailingMetadata.get() in my error handling code prior to reading it, so I'd expect that if this bug were resolved, I'd be able to properly read the data from the test client.

Thanks!

glbrntt commented 2 years ago

hey @eseay -- this is going to be deprecated when the async/await branch is merged.

Can you use a client/server with localhost networking for your tests? You will still have access to the trailingMetadata.

eseay commented 2 years ago

Can you use a client/server with localhost networking for your tests? You will still have access to the trailingMetadata.

@glbrntt Oh no! I actually started out using the async/await branch, but I swapped back to the stable release version for the sole purpose of using the mock clients since I didn't really see any negative impact to the calling semantics in comparison to the new async ones.

That's disappointing to hear. I guess I could use a mock server in my unit tests, but I would much prefer everything to be self contained in the simple swift test command without requiring additional steps before or after. The main reason I want to stub mock data into these calls is because I have logic around my requests that does transformation of the proto response models into app models.

For my classic REST networking stack, I have a "URLStubber," that's similar to some of the open source offerings but a bit more stripped down, which implements a URLProtocol to stub the requests. To your knowledge, is there anything similar to this in the SwiftNIO networking stack that I could perhaps utilize? Thanks!

glbrntt commented 2 years ago

That's disappointing to hear. I guess I could use a mock server in my unit tests, but I would much prefer everything to be self contained in the simple swift test command without requiring additional steps before or after.

I totally get that: this is absolutely a failure in the current API.

For what it's worth you can create a test server within your tests: you shouldn't need additional steps before or after swift test to do this (this is what all the grpc-swift tests involving a client and server do).

For my classic REST networking stack, I have a "URLStubber," that's similar to some of the open source offerings but a bit more stripped down, which implements a URLProtocol to stub the requests. To your knowledge, is there anything similar to this in the SwiftNIO networking stack that I could perhaps utilize? Thanks!

Not to my knowledge, I'm afraid.

eseay commented 2 years ago

For what it's worth you can create a test server within your tests: you shouldn't need additional steps before or after swift test to do this (this is what all the grpc-swift tests involving a client and server do).

Very good to know. I'll go look in there and see how it's being done, but just for the sake of this being a reference point for those who may come across it in the future - can you describe very briefly how you're going about doing that? Are you generating the gRPC server code and just invoking the server through that?

glbrntt commented 2 years ago

Yes, we're just generating the service and providing whatever implementation we need for a given test.

Each tests usually creates a new server for each test case: starting in setUp and stopping it in tearDown (as well as any EventLoopGroup and client). Sometimes it's useful to push this into a helper and start them explicitly in each test case though.

Here's an example test case: https://github.com/grpc/grpc-swift/blob/main/Tests/GRPCTests/Codegen/Normalization/NormalizationTests.swift

eseay commented 2 years ago

@glbrntt This solution worked great, and the example was perfect! Thanks for the assistance.