Repository navigation
fix(bigquery-jdbc): expand handling to OOM errors - #14759
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates exception handling across several classes, including BigQueryArrowBatchWrapper, BigQueryBaseResultSet, BigQueryFieldValueListWrapper, and BigQueryStatement, to catch and propagate Throwable instead of Exception, preventing silent stream truncation on background threads during critical failures. It also adds unit tests to verify that OutOfMemoryError is correctly propagated. The review comments provide valuable feedback, pointing out that catching Throwable without rethrowing JVM Errors (like OutOfMemoryError) is an anti-pattern that can leave the JVM in an unstable state, and suggest rethrowing these errors after they are enqueued.
| ex); | ||
| enqueueBufferError(bigQueryFieldValueListWrapperBlockingQueue, ex); | ||
| } catch (Exception ex) { | ||
| } catch (Throwable ex) { |
There was a problem hiding this comment.
Re-throw error instance?
There was a problem hiding this comment.
This is running in a background thread, thread will exit after this either way. We mark an error via the queue
Some of errors that can happen during runtime (e.g. OOM) is not an exception, but instead an Error. If that happens, results would be truncated rather than return an error to the user due to
finallyblock handling.Replacing
ExceptionwithThrowablefor handling of errors that can happen in the background threads