fix(plugin-oracle): bound the login handshake and explain native encryption stalls#1786
Conversation
…yption stalls Claude-Session: https://claude.ai/code/session_0198faM6VCrViRU4XwRoS1DC
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b372d58748
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| defer { group.cancelAll() } | ||
| guard let result = try await group.next() else { | ||
| throw TimeoutError(seconds: seconds) | ||
| } | ||
| return result |
There was a problem hiding this comment.
Make the timeout independent of child cancellation
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 a withThrowingTaskGroup waits for the still-running operation child to finish. The new test only covers Task.sleep, which is cancellable, so it misses the exact Oracle login-stall case if OracleNIO.connect is 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 👍 / 👎.
| case .connectionDropped, .connectionFailed: | ||
| return true |
There was a problem hiding this comment.
Don't map server login errors to encryption failures
When native encryption is enabled, this treats every generic .connectionFailed as a native-encryption failure. classify("server") is explicitly a .connectionFailed in the existing tests, so server-side login failures during connect (for example invalid credentials or unknown service names reported via serverDetail) can lose their real message and instead tell the user to turn encryption off. Limit this override to timeouts and known reset/dropped-handshake codes, or inspect the server detail before replacing it.
Useful? React with 👍 / 👎.
Problem
Connecting to Oracle 11.2.0.4 with the Native network encryption option turned on hangs for ~60 seconds and then fails with an unhelpful "connection reset by peer", and no diagnostic panel (#1746). The original crash in this issue is already fixed; this is the remaining "can't connect" report.
Diagnosis (from the user's packet captures)
oracleNativeEncryptionis only ever set by the user's toggle (default false; no import/SSL/migration path sets it), so this is the opt-in encryption path failing against a server that does not complete it.Two defects make this awful:
connectTimeoutbounds only the TCP connect; the TNS/ANO/auth phase has no deadline, and TablePro wrapsconnect()with none. So a stalled login hangs until the server's 60s RST..connectionFailed-> a raw IOError string with no diagnostic panel and no mention of native encryption (the one toggle that fixes it).Fix (app/plugin side only, no fork change)
withTimeouthelper inTableProPluginKit(the existingMetadataConnectionPoolreimplements this pattern inline). The Oracleconnect()now races the whole handshake against a 30s deadline, so a stall fails fast. 30s is comfortably above the ~16s a real (slow) login took in the captures and half the server's 60s.PluginDiagnosticpanel (previously this category showed none). Pure decision logic (OracleConnectErrorClassifier.isLikelyNativeEncryptionFailure) lives inTableProPluginKitand is unit-tested. No silent fallback to plaintext: the user asked for encryption, so we fail with guidance rather than overriding their choice.This does not make native encryption work on 11g (a deep fork-level ANO/AES-login fix the maintainer already deprioritized by making it opt-in). It turns a 60s silent hang into a fast, self-explanatory failure.
ABI
TableProPluginKitgainswithTimeout,TimeoutError, andisLikelyNativeEncryptionFailure(all new public symbols).scripts/check-pluginkit-abi.shshows only additions, so this is additive with nocurrentPluginKitVersionbump. Labeledabi-additive.Tests
New
TableProPluginKitTeststarget:withTimeout(stall throws within bound; fast op returns; operation errors propagate) and the encryption-failure classifier truth table. 8 pass.Shipping
Oracle is a registry-only plugin, so this reaches users after re-releasing the Oracle plugin.
Fixes #1746
https://claude.ai/code/session_0198faM6VCrViRU4XwRoS1DC