Skip to content

Commit 11f9fe9

Browse files
feat: improve aiohttp client error messages (#1201)
By default, aiohttp on bad requests will discard response body and output generic error message in response, Bad Request, Forbidden, etc. The Cloud SQL Admin APIs response body contains more detailed error messages. We need to raise these to the end user for them to be able to resolve common config issues on their own. This PR implements a more robust solution, copying the actual Cloud SQL Admin API response body's error message to the end user.
1 parent 968b6b2 commit 11f9fe9

File tree

4 files changed

+156
-72
lines changed

4 files changed

+156
-72
lines changed

google/cloud/sql/connector/client.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,19 @@ async def _get_metadata(
128128
resp = await self._client.get(url, headers=headers)
129129
if resp.status >= 500:
130130
resp = await retry_50x(self._client.get, url, headers=headers)
131-
resp.raise_for_status()
132-
ret_dict = await resp.json()
131+
# try to get response json for better error message
132+
try:
133+
ret_dict = await resp.json()
134+
if resp.status >= 400:
135+
# if detailed error message is in json response, use as error message
136+
message = ret_dict.get("error", {}).get("message")
137+
if message:
138+
resp.reason = message
139+
# skip, raise_for_status will catch all errors in finally block
140+
except Exception:
141+
pass
142+
finally:
143+
resp.raise_for_status()
133144

134145
if ret_dict["region"] != region:
135146
raise ValueError(
@@ -198,8 +209,19 @@ async def _get_ephemeral(
198209
resp = await self._client.post(url, headers=headers, json=data)
199210
if resp.status >= 500:
200211
resp = await retry_50x(self._client.post, url, headers=headers, json=data)
201-
resp.raise_for_status()
202-
ret_dict = await resp.json()
212+
# try to get response json for better error message
213+
try:
214+
ret_dict = await resp.json()
215+
if resp.status >= 400:
216+
# if detailed error message is in json response, use as error message
217+
message = ret_dict.get("error", {}).get("message")
218+
if message:
219+
resp.reason = message
220+
# skip, raise_for_status will catch all errors in finally block
221+
except Exception:
222+
pass
223+
finally:
224+
resp.raise_for_status()
203225

204226
ephemeral_cert: str = ret_dict["ephemeralCert"]["cert"]
205227

google/cloud/sql/connector/instance.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
from datetime import timezone
2323
import logging
2424

25-
import aiohttp
26-
2725
from google.cloud.sql.connector.client import CloudSQLClient
2826
from google.cloud.sql.connector.connection_info import ConnectionInfo
2927
from google.cloud.sql.connector.connection_name import _parse_instance_connection_name
@@ -128,15 +126,6 @@ async def _perform_refresh(self) -> ConnectionInfo:
128126
f"expiration = {connection_info.expiration.isoformat()}"
129127
)
130128

131-
except aiohttp.ClientResponseError as e:
132-
logger.debug(
133-
f"['{self._conn_name}']: Connection info "
134-
f"refresh operation failed: {str(e)}"
135-
)
136-
if e.status == 403:
137-
e.message = "Forbidden: Authenticated IAM principal does not seem authorized to make API request. Verify 'Cloud SQL Admin API' is enabled within your GCP project and 'Cloud SQL Client' role has been granted to IAM principal."
138-
raise
139-
140129
except Exception as e:
141130
logger.debug(
142131
f"['{self._conn_name}']: Connection info "

tests/unit/test_client.py

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
import datetime
1616
from typing import Optional
1717

18+
from aiohttp import ClientResponseError
19+
from aioresponses import aioresponses
20+
from google.auth.credentials import Credentials
1821
from mocks import FakeCredentials
1922
import pytest
2023

@@ -138,3 +141,130 @@ async def test_CloudSQLClient_user_agent(
138141
assert client._user_agent == f"cloud-sql-python-connector/{version}+{driver}"
139142
# close client
140143
await client.close()
144+
145+
146+
async def test_cloud_sql_error_messages_get_metadata(
147+
fake_credentials: Credentials,
148+
) -> None:
149+
"""
150+
Test that Cloud SQL Admin API error messages are raised for _get_metadata.
151+
"""
152+
# mock Cloud SQL Admin API calls with exceptions
153+
client = CloudSQLClient(
154+
sqladmin_api_endpoint="https://sqladmin.googleapis.com",
155+
quota_project=None,
156+
credentials=fake_credentials,
157+
)
158+
get_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance/connectSettings"
159+
resp_body = {
160+
"error": {
161+
"code": 403,
162+
"message": "Cloud SQL Admin API has not been used in project 123456789 before or it is disabled",
163+
}
164+
}
165+
with aioresponses() as mocked:
166+
mocked.get(
167+
get_url,
168+
status=403,
169+
payload=resp_body,
170+
repeat=True,
171+
)
172+
with pytest.raises(ClientResponseError) as exc_info:
173+
await client._get_metadata("my-project", "my-region", "my-instance")
174+
assert exc_info.value.status == 403
175+
assert (
176+
exc_info.value.message
177+
== "Cloud SQL Admin API has not been used in project 123456789 before or it is disabled"
178+
)
179+
await client.close()
180+
181+
182+
async def test_get_metadata_error_parsing_json(
183+
fake_credentials: Credentials,
184+
) -> None:
185+
"""
186+
Test that aiohttp default error messages are raised when _get_metadata gets
187+
a bad JSON response.
188+
"""
189+
# mock Cloud SQL Admin API calls with exceptions
190+
client = CloudSQLClient(
191+
sqladmin_api_endpoint="https://sqladmin.googleapis.com",
192+
quota_project=None,
193+
credentials=fake_credentials,
194+
)
195+
get_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance/connectSettings"
196+
resp_body = ["error"] # invalid JSON
197+
with aioresponses() as mocked:
198+
mocked.get(
199+
get_url,
200+
status=403,
201+
payload=resp_body,
202+
repeat=True,
203+
)
204+
with pytest.raises(ClientResponseError) as exc_info:
205+
await client._get_metadata("my-project", "my-region", "my-instance")
206+
assert exc_info.value.status == 403
207+
assert exc_info.value.message == "Forbidden"
208+
await client.close()
209+
210+
211+
async def test_cloud_sql_error_messages_get_ephemeral(
212+
fake_credentials: Credentials,
213+
) -> None:
214+
"""
215+
Test that Cloud SQL Admin API error messages are raised for _get_ephemeral.
216+
"""
217+
# mock Cloud SQL Admin API calls with exceptions
218+
client = CloudSQLClient(
219+
sqladmin_api_endpoint="https://sqladmin.googleapis.com",
220+
quota_project=None,
221+
credentials=fake_credentials,
222+
)
223+
post_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance:generateEphemeralCert"
224+
resp_body = {
225+
"error": {
226+
"code": 404,
227+
"message": "The Cloud SQL instance does not exist.",
228+
}
229+
}
230+
with aioresponses() as mocked:
231+
mocked.post(
232+
post_url,
233+
status=404,
234+
payload=resp_body,
235+
repeat=True,
236+
)
237+
with pytest.raises(ClientResponseError) as exc_info:
238+
await client._get_ephemeral("my-project", "my-instance", "my-key")
239+
assert exc_info.value.status == 404
240+
assert exc_info.value.message == "The Cloud SQL instance does not exist."
241+
await client.close()
242+
243+
244+
async def test_get_ephemeral_error_parsing_json(
245+
fake_credentials: Credentials,
246+
) -> None:
247+
"""
248+
Test that aiohttp default error messages are raised when _get_ephemeral gets
249+
a bad JSON response.
250+
"""
251+
# mock Cloud SQL Admin API calls with exceptions
252+
client = CloudSQLClient(
253+
sqladmin_api_endpoint="https://sqladmin.googleapis.com",
254+
quota_project=None,
255+
credentials=fake_credentials,
256+
)
257+
post_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance:generateEphemeralCert"
258+
resp_body = ["error"] # invalid JSON
259+
with aioresponses() as mocked:
260+
mocked.post(
261+
post_url,
262+
status=404,
263+
payload=resp_body,
264+
repeat=True,
265+
)
266+
with pytest.raises(ClientResponseError) as exc_info:
267+
await client._get_ephemeral("my-project", "my-instance", "my-key")
268+
assert exc_info.value.status == 404
269+
assert exc_info.value.message == "Not Found"
270+
await client.close()

tests/unit/test_instance.py

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@
1717
import asyncio
1818
import datetime
1919

20-
from aiohttp import ClientResponseError
21-
from aiohttp import RequestInfo
22-
from aioresponses import aioresponses
23-
from google.auth.credentials import Credentials
2420
from mock import patch
2521
import mocks
2622
import pytest # noqa F401 Needed to run the tests
@@ -266,59 +262,6 @@ async def test_get_preferred_ip_CloudSQLIPTypeError(cache: RefreshAheadCache) ->
266262
instance_metadata.get_preferred_ip(IPTypes.PSC)
267263

268264

269-
@pytest.mark.asyncio
270-
async def test_ClientResponseError(
271-
fake_credentials: Credentials,
272-
) -> None:
273-
"""
274-
Test that detailed error message is applied to ClientResponseError.
275-
"""
276-
# mock Cloud SQL Admin API calls with exceptions
277-
keys = asyncio.create_task(generate_keys())
278-
client = CloudSQLClient(
279-
sqladmin_api_endpoint="https://sqladmin.googleapis.com",
280-
quota_project=None,
281-
credentials=fake_credentials,
282-
)
283-
get_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance/connectSettings"
284-
post_url = "https://sqladmin.googleapis.com/sql/v1beta4/projects/my-project/instances/my-instance:generateEphemeralCert"
285-
with aioresponses() as mocked:
286-
mocked.get(
287-
get_url,
288-
status=403,
289-
exception=ClientResponseError(
290-
RequestInfo(get_url, "GET", headers=[]), history=[], status=403 # type: ignore
291-
),
292-
repeat=True,
293-
)
294-
mocked.post(
295-
post_url,
296-
status=403,
297-
exception=ClientResponseError(
298-
RequestInfo(post_url, "POST", headers=[]), history=[], status=403 # type: ignore
299-
),
300-
repeat=True,
301-
)
302-
cache = RefreshAheadCache(
303-
"my-project:my-region:my-instance",
304-
client,
305-
keys,
306-
)
307-
try:
308-
await cache._current
309-
except ClientResponseError as e:
310-
assert e.status == 403
311-
assert (
312-
e.message == "Forbidden: Authenticated IAM principal does not "
313-
"seem authorized to make API request. Verify "
314-
"'Cloud SQL Admin API' is enabled within your GCP project and "
315-
"'Cloud SQL Client' role has been granted to IAM principal."
316-
)
317-
finally:
318-
await cache.close()
319-
await client.close()
320-
321-
322265
@pytest.mark.asyncio
323266
async def test_AutoIAMAuthNotSupportedError(fake_client: CloudSQLClient) -> None:
324267
"""

0 commit comments

Comments
 (0)