-
-
Notifications
You must be signed in to change notification settings - Fork 313
fix(plugin-oracle): bound the login handshake and explain native encryption stalls #1786
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
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 |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import XCTest | ||
| @testable import TableProPluginKit | ||
|
|
||
| final class AsyncTimeoutTests: XCTestCase { | ||
| func testReturnsValueWhenOperationFinishesBeforeTimeout() async throws { | ||
| let value = try await withTimeout(seconds: 5) { 42 } | ||
| XCTAssertEqual(value, 42) | ||
| } | ||
|
|
||
| func testThrowsTimeoutWhenOperationStalls() async { | ||
| do { | ||
| _ = try await withTimeout(seconds: 0.05) { () -> Int in | ||
| try await Task.sleep(nanoseconds: 5_000_000_000) | ||
| return 1 | ||
| } | ||
| XCTFail("Expected TimeoutError") | ||
| } catch let error as TimeoutError { | ||
| XCTAssertEqual(error.seconds, 0.05) | ||
| } catch { | ||
| XCTFail("Expected TimeoutError, got \(error)") | ||
| } | ||
| } | ||
|
|
||
| func testPropagatesOperationError() async { | ||
| struct Boom: Error {} | ||
| do { | ||
| _ = try await withTimeout(seconds: 5) { () -> Int in throw Boom() } | ||
| XCTFail("Expected Boom") | ||
| } catch is Boom { | ||
| // expected | ||
| } catch { | ||
| XCTFail("Expected Boom, got \(error)") | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import XCTest | ||
| @testable import TableProPluginKit | ||
|
|
||
| final class OracleConnectErrorClassifierTests: XCTestCase { | ||
| func testClassifyKnownCodes() { | ||
| XCTAssertEqual(OracleConnectErrorClassifier.classify("uncleanShutdown"), .connectionDropped) | ||
| XCTAssertEqual(OracleConnectErrorClassifier.classify("serverVersionNotSupported"), .versionNotSupported) | ||
| XCTAssertEqual(OracleConnectErrorClassifier.classify("somethingElse"), .connectionFailed) | ||
| XCTAssertEqual( | ||
| OracleConnectErrorClassifier.classify("unsupportedVerifierType(0x12)"), | ||
| .verifierUnsupported(flag: "unsupportedVerifierType(0x12)") | ||
| ) | ||
| } | ||
|
|
||
| func testEncryptionFailureRequiresEncryptionEnabled() { | ||
| XCTAssertFalse(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .connectionFailed, nativeNetworkEncryptionEnabled: false, timedOut: true | ||
| )) | ||
| XCTAssertFalse(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .connectionDropped, nativeNetworkEncryptionEnabled: false, timedOut: false | ||
| )) | ||
| } | ||
|
|
||
| func testTimeoutWithEncryptionIsEncryptionFailure() { | ||
| XCTAssertTrue(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .connectionFailed, nativeNetworkEncryptionEnabled: true, timedOut: true | ||
| )) | ||
| } | ||
|
|
||
| func testHandshakeDropWithEncryptionIsEncryptionFailure() { | ||
| XCTAssertTrue(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .connectionDropped, nativeNetworkEncryptionEnabled: true, timedOut: false | ||
| )) | ||
| XCTAssertTrue(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .connectionFailed, nativeNetworkEncryptionEnabled: true, timedOut: false | ||
| )) | ||
| } | ||
|
|
||
| func testAuthErrorsAreNotEncryptionFailures() { | ||
| XCTAssertFalse(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .verifierUnsupported(flag: "x"), nativeNetworkEncryptionEnabled: true, timedOut: false | ||
| )) | ||
| XCTAssertFalse(OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure( | ||
| failure: .versionNotSupported, nativeNetworkEncryptionEnabled: true, timedOut: false | ||
| )) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import Foundation | ||
|
|
||
| public struct TimeoutError: Error, Sendable, Equatable { | ||
| public let seconds: Double | ||
|
|
||
| public init(seconds: Double) { | ||
| self.seconds = seconds | ||
| } | ||
| } | ||
|
|
||
| public func withTimeout<T: Sendable>( | ||
| seconds: Double, | ||
| operation: @escaping @Sendable () async throws -> T | ||
| ) async throws -> T { | ||
| try await withThrowingTaskGroup(of: T.self) { group in | ||
| group.addTask { try await operation() } | ||
| group.addTask { | ||
| try await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000)) | ||
| throw TimeoutError(seconds: seconds) | ||
| } | ||
| defer { group.cancelAll() } | ||
| guard let result = try await group.next() else { | ||
| throw TimeoutError(seconds: seconds) | ||
| } | ||
| return result | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,4 +21,19 @@ public enum OracleConnectErrorClassifier { | |
| return .connectionFailed | ||
| } | ||
| } | ||
|
|
||
| public static func isLikelyNativeEncryptionFailure( | ||
| failure: OracleConnectFailure, | ||
| nativeNetworkEncryptionEnabled: Bool, | ||
| timedOut: Bool | ||
| ) -> Bool { | ||
| guard nativeNetworkEncryptionEnabled else { return false } | ||
| if timedOut { return true } | ||
| switch failure { | ||
| case .connectionDropped, .connectionFailed: | ||
| return true | ||
|
Comment on lines
+33
to
+34
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.
When native encryption is enabled, this treats every generic Useful? React with 👍 / 👎. |
||
| case .verifierUnsupported, .versionNotSupported: | ||
| return false | ||
| } | ||
| } | ||
| } | ||
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.
For a stalled operation that does not observe task cancellation, this helper still waits past the deadline: the timeout child throws,
cancelAll()runs, but exiting awithThrowingTaskGroupwaits for the still-running operation child to finish. The new test only coversTask.sleep, which is cancellable, so it misses the exact Oracle login-stall case ifOracleNIO.connectis blocked on I/O and does not complete on cancellation; users can still wait until the server reset instead of getting the 30s error.Useful? React with 👍 / 👎.