-
Notifications
You must be signed in to change notification settings - Fork 420
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
[enhancement] GRPCChannelPool throws error from as deep as the center of the Earth #1459
Comments
It's not reasonable to exhaustively list which errors can be thrown since they may be thrown by dependencies. These dependencies can introduce new errors at any time so the goalposts are constantly moving. And, as you've identified, our dependencies can't exhaustively list errors! |
Concretely, this error is thrown from https://github.com/apple/swift-nio-ssl/blob/c30c680c78c99afdabf84805a83c8745387c4ac7/Sources/NIOSSL/SSLContext.swift#L225. This error is therefore related to the choice of signing algorithms, which would be derived from custom TLS configuration. Are you making any tweaks to the TLS configuration? |
@Lukasa That's interesting! I'm not actually customizing TLS I'm using whatever is the default for NIOSSL with this transport security configuration GRPCChannelPool
.Configuration
.TransportSecurity
.tls(
.makeClientConfigurationBackedByNIOSSL()
) then passing it to the channel constructor return try GRPCChannelPool.with(
target: ConnectionTarget.hostAndPort(host, port),
transportSecurity: transportSecurity,
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1)
) It would be great if a default config could be non-throwing! 🤩
Yes, I completely understand your point. Yet I beg to differ that maybe it could be handled in a more developer-friendly, in fact it will probably make troubleshooting easier. |
Intersting... I cannot reproduce this. What version of NIOSSL are you using?
Possibly. I think you end up loosely bucketing errors but to understand the actual root cause you need the original error. |
My Package.resolved shows
Yes. I took some time and kept cmd+clicking my way into nio-ssl to know its inner workings. Surveyed the throwing points and opened this issue on swift-nio-ssl apple/swift-nio-ssl#381 It's not that bad but it could be improved so that developers like you and me can provide a better and cleaner API. |
Hmm, no luck reproducing this with this (with 2.20.0 or other versions). As far as I understand you're essentially doing this: let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) }
XCTAssertNoThrow(try GRPCChannelPool.with(
target: .host("127.0.0.1"),
transportSecurity: .tls(.makeClientConfigurationBackedByNIOSSL()),
eventLoopGroup: group
)) Do you have a minimal reproducer for this?
🙏 |
Notice that this is a feature request for a less deep throwing API. I'm not getting any errors but it would be nice to have a friendlier API for it. I think this bubbles from NIOSSL and there's not much that swift grpc can do than some wrapping. 😥 I'll defer to the issue I created on NIOSSL repo if they improve they throwing the swift grpc will probably improve. Would it be possible to achieve a "default" setup that could be non throwing ? As @Lukasa implied? |
To be clear, it’s my understanding that you are currently using a default config. The default config grpc is using should not throw; so your current situation is something we’d like to understand |
I apologize for the misunderstanding. 🙏🙏🙏 I'm not being thrown an error. What I'm describing (and suggesting) is that the throwing originates really deep into the stack so it would be good to have more concise details of what it could cause it to throw an error to reduce the chances of users facing those issues. I could also add that if there's a known fail-proof default way that most clients will like be using, that a non-throwing API for those cases is provided. |
Is your feature request related to a problem? Please describe it.
Consider this API:
I CMD+clicked my way into the function actually doing the throwing and it seems too far fetched.
throw BoringSSLError.unknownError(errorStack)
GRPCChannel.with()
--->try PooledChannel(configuration: configuration)
--->try tlsConfiguration.makeNIOSSLContext()
---->try NIOSSLContext(configuration: configuration.configuration)
--->try self.init(configuration: configuration, callbackManager: nil, additionalCertificateChainVerification: nil)
I'm not sure what I'm supposed to catch, and what the possible errors are, but according to the last breadcrumb, there could be many and also much more deeper.
Describe the solution you'd like
Present a comprehensive list of errors that mask those occurring on dependencies of the package.
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've
considered, if any.
Additional context
Add any other context about the feature request here.
I'm researching this in the process of moving from ClientConnection to GRPCChannelPool. The latter solves many issues my users were experiencing regarding "Transport Unavailable" being thrown #1273 but it makes my service API Throwing and I can't make a sense of it.
The text was updated successfully, but these errors were encountered: