Skip to content

Commit 908ef8b

Browse files
committed
fix(bigquery): limit ssl error workaround to insert_rows
1 parent 400f465 commit 908ef8b

6 files changed

Lines changed: 98 additions & 20 deletions

File tree

packages/google-cloud-bigquery/google/cloud/bigquery/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
from google.cloud.bigquery.query import StructQueryParameter
9797
from google.cloud.bigquery.query import StructQueryParameterType
9898
from google.cloud.bigquery.query import UDFResource
99+
from google.cloud.bigquery.retry import DEFAULT_INSERT_ROWS_RETRY
99100
from google.cloud.bigquery.retry import DEFAULT_RETRY
100101
from google.cloud.bigquery.routine import DeterminismLevel
101102
from google.cloud.bigquery.routine import Routine
@@ -204,6 +205,7 @@
204205
"ParquetOptions",
205206
"ScriptOptions",
206207
"TransactionInfo",
208+
"DEFAULT_INSERT_ROWS_RETRY",
207209
"DEFAULT_RETRY",
208210
# Standard SQL types
209211
"StandardSqlDataType",

packages/google-cloud-bigquery/google/cloud/bigquery/client.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
from google.cloud.bigquery.query import _QueryResults
112112
from google.cloud.bigquery.retry import (
113113
DEFAULT_GET_JOB_TIMEOUT,
114+
DEFAULT_INSERT_ROWS_RETRY,
114115
DEFAULT_JOB_RETRY,
115116
DEFAULT_RETRY,
116117
DEFAULT_TIMEOUT,
@@ -3956,7 +3957,7 @@ def insert_rows_json(
39563957
skip_invalid_rows: Optional[bool] = None,
39573958
ignore_unknown_values: Optional[bool] = None,
39583959
template_suffix: Optional[str] = None,
3959-
retry: retries.Retry = DEFAULT_RETRY,
3960+
retry: retries.Retry = DEFAULT_INSERT_ROWS_RETRY,
39603961
timeout: TimeoutType = DEFAULT_TIMEOUT,
39613962
) -> Sequence[dict]:
39623963
"""Insert rows into a table without applying local type conversions.
@@ -4080,6 +4081,9 @@ def insert_rows_json(
40804081
path = "%s/insertAll" % table.path
40814082
# We can always retry, because every row has an insert ID.
40824083
span_attributes = {"path": path}
4084+
if retry is DEFAULT_RETRY:
4085+
retry = DEFAULT_INSERT_ROWS_RETRY
4086+
40834087
try:
40844088
response = self._call_api(
40854089
retry,
@@ -4090,12 +4094,15 @@ def insert_rows_json(
40904094
data=data,
40914095
timeout=timeout,
40924096
)
4093-
except requests.exceptions.SSLError as exc:
4094-
msg = (
4095-
"An SSL/Connection error occurred while streaming rows. This "
4096-
"could be due to an invalid request (e.g., invalid table schema)."
4097-
)
4098-
raise requests.exceptions.SSLError(msg) from exc
4097+
except (requests.exceptions.SSLError, core_exceptions.RetryError) as exc:
4098+
cause = exc.cause if isinstance(exc, core_exceptions.RetryError) else exc
4099+
if isinstance(cause, requests.exceptions.SSLError):
4100+
msg = (
4101+
"An SSL/Connection error occurred while streaming rows. This "
4102+
"could be due to an invalid request (e.g., invalid table schema)."
4103+
)
4104+
raise requests.exceptions.SSLError(msg) from exc
4105+
raise
40994106
errors = []
41004107

41014108
for error in response.get("insertErrors", ()):

packages/google-cloud-bigquery/google/cloud/bigquery/retry.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,6 @@
4040

4141
_DEFAULT_RETRY_DEADLINE = 10.0 * 60.0 # 10 minutes
4242

43-
# Exceptions that are subclasses of types in _UNSTRUCTURED_RETRYABLE_TYPES
44-
# but should not be retried because they typically indicate persistent
45-
# configuration or security issues.
46-
_UNSTRUCTURED_NON_RETRYABLE_TYPES = (requests.exceptions.SSLError,)
47-
4843
# Ambiguous errors (e.g. internalError, backendError, rateLimitExceeded) retry
4944
# until the full `_DEFAULT_RETRY_DEADLINE`. This is because the
5045
# `jobs.getQueryResults` REST API translates a job failure into an HTTP error.
@@ -69,13 +64,9 @@
6964
def _should_retry(exc):
7065
"""Predicate for determining when to retry.
7166
72-
We retry if the 'reason' is in _RETRYABLE_REASONS or if the exception
73-
is an instance of one of the _UNSTRUCTURED_RETRYABLE_TYPES, unless it
74-
is explicitly excluded by being in _UNSTRUCTURED_NON_RETRYABLE_TYPES.
67+
We retry if and only if the 'reason' is in _RETRYABLE_REASONS or is
68+
in _UNSTRUCTURED_RETRYABLE_TYPES.
7569
"""
76-
if isinstance(exc, _UNSTRUCTURED_NON_RETRYABLE_TYPES):
77-
return False
78-
7970
try:
8071
reason = exc.errors[0]["reason"]
8172
except (AttributeError, IndexError, TypeError, KeyError):
@@ -98,6 +89,23 @@ def _should_retry(exc):
9889
"""
9990

10091

92+
def _should_retry_insert_rows(exc):
93+
"""Predicate for determining when to retry streaming inserts (insertAll).
94+
95+
Unlike standard API calls, tabledata.insertAll failures due to schema
96+
mismatches often manifest as an SSLError because the server abruptly terminates
97+
the connection. These errors will not resolve on retry and should fail
98+
immediately with descriptive guidance.
99+
"""
100+
if isinstance(exc, requests.exceptions.SSLError):
101+
return False
102+
return _should_retry(exc)
103+
104+
105+
DEFAULT_INSERT_ROWS_RETRY = DEFAULT_RETRY.with_predicate(_should_retry_insert_rows)
106+
"""The default retry object for streaming inserts (insert_rows_json / insert_rows)."""
107+
108+
101109
def _should_retry_get_job_conflict(exc):
102110
"""Predicate for determining when to retry a jobs.get call after a conflict error.
103111

packages/google-cloud-bigquery/tests/system/test_ssl_retry.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def mock_api_request(*args, **kwargs):
4444
bigquery_client._connection, "api_request", side_effect=mock_api_request
4545
):
4646
# Use a reasonably short deadline for the test, although it should fail on the first attempt anyway.
47-
retry = bigquery.DEFAULT_RETRY.with_deadline(5.0)
47+
retry = bigquery.DEFAULT_INSERT_ROWS_RETRY.with_deadline(5.0)
4848

4949
start_time = time.time()
5050
with pytest.raises(requests.exceptions.SSLError) as excinfo:

packages/google-cloud-bigquery/tests/unit/test_client.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6898,6 +6898,38 @@ def test_insert_rows_json_w_ssl_error(self):
68986898
self.assertIn("invalid table schema", str(context.exception))
68996899
self.assertIn("SSL/Connection error occurred", str(context.exception))
69006900

6901+
def test_insert_rows_json_w_ssl_error_explicit_default_retry(self):
6902+
import requests.exceptions
6903+
from google.cloud.bigquery.dataset import DatasetReference
6904+
from google.cloud.bigquery.schema import SchemaField
6905+
from google.cloud.bigquery.table import Table
6906+
from google.cloud.bigquery.retry import DEFAULT_RETRY
6907+
6908+
PROJECT = "PROJECT"
6909+
DS_ID = "DS_ID"
6910+
TABLE_ID = "TABLE_ID"
6911+
ROWS = [{"full_name": "Bhettye Rhubble", "age": "27", "joined": None}]
6912+
6913+
creds = _make_credentials()
6914+
client = self._make_one(project=PROJECT, credentials=creds, _http=object())
6915+
conn = client._connection = make_connection({})
6916+
6917+
conn.api_request.side_effect = requests.exceptions.SSLError("EOF occurred")
6918+
6919+
table_ref = DatasetReference(PROJECT, DS_ID).table(TABLE_ID)
6920+
schema = [
6921+
SchemaField("full_name", "STRING", mode="REQUIRED"),
6922+
SchemaField("age", "INTEGER", mode="REQUIRED"),
6923+
SchemaField("joined", "TIMESTAMP", mode="NULLABLE"),
6924+
]
6925+
table = Table(table_ref, schema=schema)
6926+
6927+
with self.assertRaises(requests.exceptions.SSLError) as context:
6928+
client.insert_rows_json(table, ROWS, retry=DEFAULT_RETRY)
6929+
6930+
self.assertIn("invalid table schema", str(context.exception))
6931+
self.assertIn("SSL/Connection error occurred", str(context.exception))
6932+
69016933
def test_list_partitions(self):
69026934
from google.cloud.bigquery.table import Table
69036935

packages/google-cloud-bigquery/tests/unit/test_retry.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def test_w_unstructured_requests_connectionerror(self):
5353

5454
def test_w_unstructured_requests_sslerror(self):
5555
exc = requests.exceptions.SSLError()
56-
self.assertFalse(self._call_fut(exc))
56+
self.assertTrue(self._call_fut(exc))
5757

5858
def test_w_unstructured_requests_chunked_encoding_error(self):
5959
exc = requests.exceptions.ChunkedEncodingError()
@@ -160,3 +160,32 @@ def test_DEFAULT_JOB_RETRY_job_rate_limit_exceeded_retry_predicate():
160160
assert DEFAULT_JOB_RETRY._predicate(
161161
ClientError("fail", errors=[dict(reason="backendError")])
162162
)
163+
164+
165+
class Test_should_retry_insert_rows(unittest.TestCase):
166+
def _call_fut(self, exc):
167+
from google.cloud.bigquery.retry import _should_retry_insert_rows
168+
169+
return _should_retry_insert_rows(exc)
170+
171+
def test_w_unstructured_requests_sslerror(self):
172+
exc = requests.exceptions.SSLError()
173+
self.assertFalse(self._call_fut(exc))
174+
175+
def test_w_unstructured_requests_connectionerror(self):
176+
exc = requests.exceptions.ConnectionError()
177+
self.assertTrue(self._call_fut(exc))
178+
179+
def test_w_backendError(self):
180+
exc = mock.Mock(errors=[{"reason": "backendError"}], spec=["errors"])
181+
self.assertTrue(self._call_fut(exc))
182+
183+
184+
def test_DEFAULT_INSERT_ROWS_RETRY_predicate():
185+
from google.cloud.bigquery.retry import DEFAULT_INSERT_ROWS_RETRY
186+
187+
exc_ssl = requests.exceptions.SSLError()
188+
assert not DEFAULT_INSERT_ROWS_RETRY._predicate(exc_ssl)
189+
190+
exc_conn = requests.exceptions.ConnectionError()
191+
assert DEFAULT_INSERT_ROWS_RETRY._predicate(exc_conn)

0 commit comments

Comments
 (0)