-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to v1.0.0-rc3 of the conformance suite #226
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,32 @@ | ||
# Test runner is overly strict in asserting the contents | ||
# of the message query param, so these tests currently fail. | ||
Idempotency/** | ||
# If error body is JSON null, it is interpreted as an unknown error | ||
# instead of falling back to basing code on the HTTP status. | ||
Connect Error and End-Stream/**/error/null | ||
|
||
# Deadline headers are not currently set. | ||
Deadline Propagation/** | ||
|
||
# This response doesn't look like a normal Connect response, so it | ||
# goes through okhttp's default 408 code handling, which retries. | ||
# The retry triggers an error: | ||
# client sent another request (#2) for the same test case | ||
HTTP to Connect Code Mapping/**/request-timeout | ||
|
||
# Bug: response content-type is not correctly checked | ||
Unexpected Responses/**/unexpected-content-type | ||
|
||
# Bug: "trailers-only" responses are not correctly identified. | ||
# If headers contain "grpc-status", this client assumes it is a | ||
# trailers-only response. However, a trailers-only response should | ||
# instead be identified by lack of body or HTTP trailers. | ||
gRPC Unexpected Responses/**/trailers-only/* | ||
|
||
# Bug: if gRPC unary response contains zero messages or | ||
# more than one message, client does not complain if | ||
# status trailers says "ok" | ||
gRPC Unexpected Responses/**/unary-multiple-responses | ||
gRPC Unexpected Responses/**/unary-ok-but-no-response | ||
gRPC-Web Unexpected Responses/**/unary-ok-but-no-response | ||
|
||
# Bug: incorrect code attribution for these failures (INTERNAL instead of UNKNOWN) | ||
gRPC-Web Unexpected Responses/**/missing-status | ||
gRPC-Web Unexpected Responses/**/trailers-in-body/unary-multiple-responses |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,13 +14,13 @@ | |
|
||
package com.connectrpc.conformance.client | ||
|
||
import com.connectrpc.Code | ||
import com.connectrpc.ConnectException | ||
import com.connectrpc.Headers | ||
import com.connectrpc.ProtocolClientConfig | ||
import com.connectrpc.RequestCompression | ||
import com.connectrpc.ResponseMessage | ||
import com.connectrpc.SerializationStrategy | ||
import com.connectrpc.asConnectException | ||
import com.connectrpc.compression.GzipCompressionPool | ||
import com.connectrpc.conformance.client.adapt.AnyMessage | ||
import com.connectrpc.conformance.client.adapt.BidiStreamClient | ||
|
@@ -228,17 +228,8 @@ class Client( | |
} | ||
} catch (ex: Throwable) { | ||
if (!sent) { | ||
val connEx = if (ex is ConnectException) { | ||
ex | ||
} else { | ||
ConnectException( | ||
code = Code.UNKNOWN, | ||
message = ex.message, | ||
exception = ex, | ||
) | ||
} | ||
return ClientResponseResult( | ||
error = connEx, | ||
error = asConnectException(ex), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I consolidated a few |
||
numUnsentRequests = 1, | ||
) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,9 +27,11 @@ import com.connectrpc.ServerOnlyStreamInterface | |
import com.connectrpc.StreamResult | ||
import com.connectrpc.StreamType | ||
import com.connectrpc.UnaryBlockingCall | ||
import com.connectrpc.asConnectException | ||
import com.connectrpc.http.Cancelable | ||
import com.connectrpc.http.HTTPClientInterface | ||
import com.connectrpc.http.HTTPRequest | ||
import com.connectrpc.http.HTTPResponse | ||
import com.connectrpc.http.UnaryHTTPRequest | ||
import com.connectrpc.http.dispatchIn | ||
import com.connectrpc.http.transform | ||
|
@@ -94,7 +96,20 @@ class ProtocolClient( | |
val unaryFunc = config.createInterceptorChain() | ||
val finalRequest = unaryFunc.requestFunction(unaryRequest) | ||
val cancelable = httpClient.unary(finalRequest) httpClientUnary@{ httpResponse -> | ||
val finalResponse = unaryFunc.responseFunction(httpResponse) | ||
val finalResponse: HTTPResponse | ||
try { | ||
finalResponse = unaryFunc.responseFunction(httpResponse) | ||
} catch (ex: Throwable) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was one of the previously uncaught exceptions. It was happening when there was a problem in the wire format of the 5-byte envelopes around stream messages. Instead of being turned into an RPC failure, it was causing an okhttp worker thread to die, which would then cause the conformance client to hang and timeout. |
||
val connEx = asConnectException(ex) | ||
onResult( | ||
ResponseMessage.Failure( | ||
connEx, | ||
emptyMap(), | ||
connEx.metadata, | ||
), | ||
) | ||
return@httpClientUnary | ||
} | ||
val exception = finalResponse.cause?.setErrorParser(serializationStrategy.errorDetailParser()) | ||
if (exception != null) { | ||
onResult( | ||
|
@@ -110,10 +125,10 @@ class ProtocolClient( | |
val responseMessage: Output | ||
try { | ||
responseMessage = responseCodec.deserialize(finalResponse.message) | ||
} catch (e: Exception) { | ||
} catch (ex: Exception) { | ||
onResult( | ||
ResponseMessage.Failure( | ||
ConnectException(code = Code.INTERNAL_ERROR, exception = e), | ||
asConnectException(ex, Code.INTERNAL_ERROR), | ||
finalResponse.headers, | ||
finalResponse.trailers, | ||
), | ||
|
@@ -129,8 +144,16 @@ class ProtocolClient( | |
) | ||
} | ||
return cancelable | ||
} catch (e: Exception) { | ||
throw e | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the other uncaught exception. 🤦 |
||
} catch (ex: Exception) { | ||
val connEx = asConnectException(ex) | ||
onResult( | ||
ResponseMessage.Failure( | ||
connEx, | ||
emptyMap(), | ||
connEx.metadata, | ||
), | ||
) | ||
return { } | ||
} | ||
} | ||
|
||
|
@@ -228,12 +251,7 @@ class ProtocolClient( | |
try { | ||
streamResult = streamFunc.streamResultFunction(initialResult) | ||
} catch (ex: Throwable) { | ||
val connEx = if (ex is ConnectException) { | ||
ex | ||
} else { | ||
ConnectException(code = Code.UNKNOWN, exception = ex) | ||
} | ||
streamResult = StreamResult.Complete(connEx) | ||
streamResult = StreamResult.Complete(asConnectException(ex)) | ||
} | ||
when (streamResult) { | ||
is StreamResult.Headers -> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these cleanup calls because we removed an unused message from the conformance protos, and it was causing a compile error until I did this due to an old, invalid source file being left around in the output folder.