-
Notifications
You must be signed in to change notification settings - Fork 420
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Motivation: When the child channel initializer fails, or if any part of child channel setup fails, the error that results is not thrown into that child channel but is instead propagated into the server channel. As nothing in grpc was listening for errors there, those errors got lost. This can make some problems particularly tricky to debug, as they lead to silent service failures. Modifications: - Added a channel handler to be put in all server channels. Results: Server channel creation errors now get propagated into the error delegate.
- Loading branch information
Showing
5 changed files
with
209 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Copyright 2020, gRPC Authors All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
import NIO | ||
|
||
/// A handler that passes errors thrown into the server channel to the server error delegate. | ||
/// | ||
/// A NIO server bootstrap produces two kinds of channels. The first and most common is the "child" channel: | ||
/// each of these corresponds to one connection, and has the connection state stored on it. The other kind is | ||
/// the "server" channel. Each bootstrap produces only one of these, and it is the channel that owns the listening | ||
/// socket. | ||
/// | ||
/// This channel handler is inserted into the server channel, and is responsible for passing any errors in that pipeline | ||
/// to the server error delegate. If there is no error delegate, this handler is not inserted into the pipeline. | ||
final class ServerChannelErrorHandler { | ||
private let errorDelegate: ServerErrorDelegate | ||
|
||
init(errorDelegate: ServerErrorDelegate) { | ||
self.errorDelegate = errorDelegate | ||
} | ||
} | ||
|
||
extension ServerChannelErrorHandler: ChannelInboundHandler { | ||
typealias InboundIn = Any | ||
typealias InboundOut = Any | ||
|
||
func errorCaught(context: ChannelHandlerContext, error: Error) { | ||
// This handler does not treat errors as fatal to the listening socket, as it's possible they were transiently | ||
// occurring in a single connection setup attempt. | ||
errorDelegate.observeLibraryError(error) | ||
context.fireErrorCaught(error) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
/* | ||
* Copyright 2020, gRPC Authors All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
@testable import GRPC | ||
import GRPCSampleData | ||
import EchoImplementation | ||
import Logging | ||
import NIO | ||
import NIOSSL | ||
import XCTest | ||
|
||
class ServerErrorRecordingDelegate: ServerErrorDelegate { | ||
var errors: [Error] = [] | ||
var expectation: XCTestExpectation | ||
|
||
init(expectation: XCTestExpectation) { | ||
self.expectation = expectation | ||
} | ||
|
||
func observeLibraryError(_ error: Error) { | ||
self.errors.append(error) | ||
self.expectation.fulfill() | ||
} | ||
} | ||
|
||
class ServerTLSErrorTests: GRPCTestCase { | ||
let defaultClientTLSConfiguration = ClientConnection.Configuration.TLS( | ||
certificateChain: [.certificate(SampleCertificate.client.certificate)], | ||
privateKey: .privateKey(SamplePrivateKey.client), | ||
trustRoots: .certificates([SampleCertificate.ca.certificate]), | ||
hostnameOverride: SampleCertificate.server.commonName) | ||
|
||
var defaultTestTimeout: TimeInterval = 1.0 | ||
|
||
var clientEventLoopGroup: EventLoopGroup! | ||
var serverEventLoopGroup: EventLoopGroup! | ||
|
||
func makeClientConfiguration( | ||
tls: ClientConnection.Configuration.TLS, | ||
port: Int | ||
) -> ClientConnection.Configuration { | ||
return .init( | ||
target: .hostAndPort("localhost", port), | ||
eventLoopGroup: self.clientEventLoopGroup, | ||
tls: tls, | ||
// No need to retry connecting. | ||
connectionBackoff: nil | ||
) | ||
} | ||
|
||
func makeClientConnectionExpectation() -> XCTestExpectation { | ||
return self.expectation(description: "EventLoopFuture<ClientConnection> resolved") | ||
} | ||
|
||
override func setUp() { | ||
self.serverEventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) | ||
self.clientEventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) | ||
} | ||
|
||
override func tearDown() { | ||
XCTAssertNoThrow(try self.clientEventLoopGroup.syncShutdownGracefully()) | ||
self.clientEventLoopGroup = nil | ||
|
||
XCTAssertNoThrow(try self.serverEventLoopGroup.syncShutdownGracefully()) | ||
self.serverEventLoopGroup = nil | ||
} | ||
|
||
func testErrorIsLoggedWhenSSLContextErrors() throws { | ||
let clientShutdownExpectation = self.expectation(description: "client shutdown") | ||
let errorExpectation = self.expectation(description: "error") | ||
let errorDelegate = ServerErrorRecordingDelegate(expectation: errorExpectation) | ||
|
||
let server = try! Server.secure( | ||
group: self.serverEventLoopGroup, | ||
certificateChain: [SampleCertificate.exampleServerWithExplicitCurve.certificate], | ||
privateKey: SamplePrivateKey.exampleServerWithExplicitCurve | ||
).withServiceProviders([EchoProvider()]) | ||
.withErrorDelegate(errorDelegate) | ||
.bind(host: "localhost", port: 0) | ||
.wait() | ||
defer { | ||
XCTAssertNoThrow(try server.close().wait()) | ||
} | ||
|
||
let port = server.channel.localAddress!.port! | ||
|
||
var tls = self.defaultClientTLSConfiguration | ||
tls.trustRoots = .certificates([SampleCertificate.exampleServerWithExplicitCurve.certificate]) | ||
var configuration = self.makeClientConfiguration(tls: tls, port: port) | ||
|
||
let stateChangeDelegate = ConnectivityStateCollectionDelegate(shutdown: clientShutdownExpectation) | ||
configuration.connectivityStateDelegate = stateChangeDelegate | ||
|
||
_ = ClientConnection(configuration: configuration) | ||
|
||
self.wait(for: [clientShutdownExpectation, errorExpectation], timeout: self.defaultTestTimeout) | ||
|
||
if let nioSSLError = errorDelegate.errors.first as? NIOSSLError, | ||
case .failedToLoadCertificate = nioSSLError { | ||
// Expected case. | ||
} else { | ||
XCTFail("Expected NIOSSLError.handshakeFailed(BoringSSL.sslError)") | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters