dongjoon-hyun opened a new pull request, #507: URL: https://github.com/apache/spark-connect-swift/pull/507
### What changes were proposed in this pull request? This PR replaces force unwraps on server-provided response arrays with an explicit `throw`, so an empty server response surfaces as a Swift error instead of trapping the process. A single internal helper, `Array.firstOrThrow()`, is added to `Extension.swift`. It throws `SparkConnectError.invalidState` on an empty array and uses a `#function` default argument so the error message names the failing operation without any per-call-site boilerplate. 31 call sites are converted across `Catalog` (15), `StreamingQuery` (7), `StreamingQueryManager` (3), `SparkConnectClient` (3), `DataFrameStatFunctions` (2), and `DataStreamWriter` (1). Both spellings of the same trap are covered, because `Catalog` uses them interchangeably for the same operation: `response.first!` on `[ExecutePlanResponse]`, and `df.collect()[0]` / `df.collect().first!` on `[Row]`. ### Why are the changes needed? `.first!` on a server response array traps and kills the process when the Spark Connect server returns an empty response stream. A misbehaving or incompatible server should not be able to crash a client application; it should raise a catchable error. This is the same class of protocol violation that `SparkConnectClient+ReattachableExecute` already handles by throwing `invalidState` when the server side session ID changes, so this PR follows that precedent instead of adding a new error case. Practically every public accessor of `StreamingQuery` was affected, as well as the main SQL execution path in `SparkConnectClient.getExecuteExternalCommand`. ### Does this PR introduce _any_ user-facing change? Yes, in the failure path only. Where the client previously trapped on an empty server response, it now throws `SparkConnectError.invalidState`: ``` The server returned an empty response for getDatabase(_:). ``` There is no behavior change when the server responds normally. ### How was this patch tested? Pass the CIs with a new `ExtensionTests` suite covering `firstOrThrow()`, which requires no Spark Connect server. The remaining call sites cannot be unit-tested without a server that returns an empty response stream, so they are covered by the existing integration tests. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
