Skip to content

Commit 6cddd8c

Browse files
Robbie-PalmerRobbie-Palmer
committed
fix: Replace bare except blocks with specific exception types
Replace bare `except:` with specific exception tuple `except (TypeError, ValueError, AttributeError, KeyError):` in union property parsing template. This improves code quality by: - Making error handling more explicit and intentional - Avoiding catching system exceptions like KeyboardInterrupt or SystemExit - Removing the need for noqa: E722 linting suppressions - Making the code comply with PEP 8 style guidelines All generated models with union properties now use specific exception handling in their _parse functions.
1 parent a6a636c commit 6cddd8c

File tree

12 files changed

+188
-36
lines changed

12 files changed

+188
-36
lines changed

‎end_to_end_tests/functional_tests/generated_code_execution/test_path_parameters.py‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
from unittest.mock import MagicMock
2+
3+
import httpx
4+
import pytest
5+
6+
from end_to_end_tests.functional_tests.helpers import (
7+
with_generated_client_fixture,
8+
with_generated_code_imports,
9+
)
10+
11+
12+
@with_generated_client_fixture(
13+
"""
14+
paths:
15+
/organizations/{organization}/resources/{resource_id}:
16+
get:
17+
operationId: getResource
18+
parameters:
19+
- name: organization
20+
in: path
21+
required: true
22+
schema:
23+
type: string
24+
- name: resource_id
25+
in: path
26+
required: true
27+
schema:
28+
type: string
29+
responses:
30+
"200":
31+
description: Success
32+
content:
33+
application/json:
34+
schema:
35+
type: object
36+
properties:
37+
id:
38+
type: string
39+
"""
40+
)
41+
@with_generated_code_imports(
42+
".api.default.get_resource.sync_detailed",
43+
".client.Client",
44+
)
45+
class TestPathParameterEncoding:
46+
def test_path_params_with_reserved_chars_are_encoded(self, sync_detailed, Client):
47+
"""Test that path parameters with reserved characters are properly URL-encoded"""
48+
# Create a mock httpx client
49+
mock_httpx_client = MagicMock(spec=httpx.Client)
50+
mock_response = MagicMock(spec=httpx.Response)
51+
mock_response.status_code = 200
52+
mock_response.json.return_value = {"id": "test"}
53+
mock_response.content = b'{"id": "test"}'
54+
mock_response.headers = {}
55+
mock_httpx_client.request.return_value = mock_response
56+
57+
# Create a client with the mock httpx client
58+
client = Client(base_url="https://api.example.com")
59+
client.set_httpx_client(mock_httpx_client)
60+
61+
# Call the endpoint with path parameters containing reserved characters
62+
sync_detailed(
63+
organization="org/with/slashes",
64+
resource_id="id?with=query&chars",
65+
client=client,
66+
)
67+
68+
# Verify the request was made with properly encoded URL
69+
mock_httpx_client.request.assert_called_once()
70+
call_kwargs = mock_httpx_client.request.call_args[1]
71+
72+
# The URL should have encoded slashes and query characters
73+
expected_url = "/organizations/org%2Fwith%2Fslashes/resources/id%3Fwith%3Dquery%26chars"
74+
assert call_kwargs["url"] == expected_url
75+
76+
def test_path_params_with_spaces_are_encoded(self, sync_detailed, Client):
77+
"""Test that path parameters with spaces are properly URL-encoded"""
78+
mock_httpx_client = MagicMock(spec=httpx.Client)
79+
mock_response = MagicMock(spec=httpx.Response)
80+
mock_response.status_code = 200
81+
mock_response.json.return_value = {"id": "test"}
82+
mock_response.content = b'{"id": "test"}'
83+
mock_response.headers = {}
84+
mock_httpx_client.request.return_value = mock_response
85+
86+
client = Client(base_url="https://api.example.com")
87+
client.set_httpx_client(mock_httpx_client)
88+
89+
sync_detailed(
90+
organization="org with spaces",
91+
resource_id="id with spaces",
92+
client=client,
93+
)
94+
95+
mock_httpx_client.request.assert_called_once()
96+
call_kwargs = mock_httpx_client.request.call_args[1]
97+
98+
# Spaces should be encoded as %20
99+
expected_url = "/organizations/org%20with%20spaces/resources/id%20with%20spaces"
100+
assert call_kwargs["url"] == expected_url
101+
102+
def test_path_params_with_hash_are_encoded(self, sync_detailed, Client):
103+
"""Test that path parameters with hash/fragment characters are properly URL-encoded"""
104+
mock_httpx_client = MagicMock(spec=httpx.Client)
105+
mock_response = MagicMock(spec=httpx.Response)
106+
mock_response.status_code = 200
107+
mock_response.json.return_value = {"id": "test"}
108+
mock_response.content = b'{"id": "test"}'
109+
mock_response.headers = {}
110+
mock_httpx_client.request.return_value = mock_response
111+
112+
client = Client(base_url="https://api.example.com")
113+
client.set_httpx_client(mock_httpx_client)
114+
115+
sync_detailed(
116+
organization="org#1",
117+
resource_id="resource#id",
118+
client=client,
119+
)
120+
121+
mock_httpx_client.request.assert_called_once()
122+
call_kwargs = mock_httpx_client.request.call_args[1]
123+
124+
# Hash should be encoded as %23
125+
expected_url = "/organizations/org%231/resources/resource%23id"
126+
assert call_kwargs["url"] == expected_url
127+
128+
def test_path_params_normal_chars_still_work(self, sync_detailed, Client):
129+
"""Test that normal path parameters without special characters still work"""
130+
mock_httpx_client = MagicMock(spec=httpx.Client)
131+
mock_response = MagicMock(spec=httpx.Response)
132+
mock_response.status_code = 200
133+
mock_response.json.return_value = {"id": "test"}
134+
mock_response.content = b'{"id": "test"}'
135+
mock_response.headers = {}
136+
mock_httpx_client.request.return_value = mock_response
137+
138+
client = Client(base_url="https://api.example.com")
139+
client.set_httpx_client(mock_httpx_client)
140+
141+
sync_detailed(
142+
organization="my-org",
143+
resource_id="resource-123",
144+
client=client,
145+
)
146+
147+
mock_httpx_client.request.assert_called_once()
148+
call_kwargs = mock_httpx_client.request.call_args[1]
149+
150+
# Normal characters (alphanumeric, hyphens) should still be encoded but look the same
151+
expected_url = "/organizations/my-org/resources/resource-123"
152+
assert call_kwargs["url"] == expected_url

‎end_to_end_tests/golden-record/my_test_api_client/api/default/get_models_oneof_with_required_const.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def _parse_response_200(
3737
response_200_type_0 = GetModelsOneofWithRequiredConstResponse200Type0.from_dict(data)
3838

3939
return response_200_type_0
40-
except: # noqa: E722
40+
except (TypeError, ValueError, AttributeError, KeyError):
4141
pass
4242
if not isinstance(data, dict):
4343
raise TypeError()

‎end_to_end_tests/golden-record/my_test_api_client/models/a_model.py‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ def _parse_a_camel_date_time(data: object) -> datetime.date | datetime.datetime:
272272
a_camel_date_time_type_0 = isoparse(data)
273273

274274
return a_camel_date_time_type_0
275-
except: # noqa: E722
275+
except (TypeError, ValueError, AttributeError, KeyError):
276276
pass
277277
if not isinstance(data, str):
278278
raise TypeError()
@@ -293,7 +293,7 @@ def _parse_a_nullable_date(data: object) -> datetime.date | None:
293293
a_nullable_date_type_0 = isoparse(data).date()
294294

295295
return a_nullable_date_type_0
296-
except: # noqa: E722
296+
except (TypeError, ValueError, AttributeError, KeyError):
297297
pass
298298
return cast(datetime.date | None, data)
299299

@@ -310,7 +310,7 @@ def _parse_a_nullable_uuid(data: object) -> None | UUID:
310310
a_nullable_uuid_type_0 = UUID(data)
311311

312312
return a_nullable_uuid_type_0
313-
except: # noqa: E722
313+
except (TypeError, ValueError, AttributeError, KeyError):
314314
pass
315315
return cast(None | UUID, data)
316316

@@ -332,15 +332,15 @@ def _parse_one_of_models(data: object) -> Any | FreeFormModel | ModelWithUnionPr
332332
one_of_models_type_0 = FreeFormModel.from_dict(data)
333333

334334
return one_of_models_type_0
335-
except: # noqa: E722
335+
except (TypeError, ValueError, AttributeError, KeyError):
336336
pass
337337
try:
338338
if not isinstance(data, dict):
339339
raise TypeError()
340340
one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
341341

342342
return one_of_models_type_1
343-
except: # noqa: E722
343+
except (TypeError, ValueError, AttributeError, KeyError):
344344
pass
345345
return cast(Any | FreeFormModel | ModelWithUnionProperty, data)
346346

@@ -355,15 +355,15 @@ def _parse_nullable_one_of_models(data: object) -> FreeFormModel | ModelWithUnio
355355
nullable_one_of_models_type_0 = FreeFormModel.from_dict(data)
356356

357357
return nullable_one_of_models_type_0
358-
except: # noqa: E722
358+
except (TypeError, ValueError, AttributeError, KeyError):
359359
pass
360360
try:
361361
if not isinstance(data, dict):
362362
raise TypeError()
363363
nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
364364

365365
return nullable_one_of_models_type_1
366-
except: # noqa: E722
366+
except (TypeError, ValueError, AttributeError, KeyError):
367367
pass
368368
return cast(FreeFormModel | ModelWithUnionProperty | None, data)
369369

@@ -380,7 +380,7 @@ def _parse_nullable_model(data: object) -> ModelWithUnionProperty | None:
380380
nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
381381

382382
return nullable_model_type_1
383-
except: # noqa: E722
383+
except (TypeError, ValueError, AttributeError, KeyError):
384384
pass
385385
return cast(ModelWithUnionProperty | None, data)
386386

@@ -445,7 +445,7 @@ def _parse_not_required_one_of_models(data: object) -> FreeFormModel | ModelWith
445445
not_required_one_of_models_type_0 = FreeFormModel.from_dict(data)
446446

447447
return not_required_one_of_models_type_0
448-
except: # noqa: E722
448+
except (TypeError, ValueError, AttributeError, KeyError):
449449
pass
450450
if not isinstance(data, dict):
451451
raise TypeError()
@@ -468,15 +468,15 @@ def _parse_not_required_nullable_one_of_models(
468468
not_required_nullable_one_of_models_type_0 = FreeFormModel.from_dict(data)
469469

470470
return not_required_nullable_one_of_models_type_0
471-
except: # noqa: E722
471+
except (TypeError, ValueError, AttributeError, KeyError):
472472
pass
473473
try:
474474
if not isinstance(data, dict):
475475
raise TypeError()
476476
not_required_nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
477477

478478
return not_required_nullable_one_of_models_type_1
479-
except: # noqa: E722
479+
except (TypeError, ValueError, AttributeError, KeyError):
480480
pass
481481
return cast(FreeFormModel | ModelWithUnionProperty | None | str | Unset, data)
482482

@@ -502,7 +502,7 @@ def _parse_not_required_nullable_model(data: object) -> ModelWithUnionProperty |
502502
not_required_nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
503503

504504
return not_required_nullable_model_type_1
505-
except: # noqa: E722
505+
except (TypeError, ValueError, AttributeError, KeyError):
506506
pass
507507
return cast(ModelWithUnionProperty | None | Unset, data)
508508

‎end_to_end_tests/golden-record/my_test_api_client/models/body_upload_file_tests_upload_post.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ def _parse_some_nullable_object(data: object) -> BodyUploadFileTestsUploadPostSo
276276
some_nullable_object_type_0 = BodyUploadFileTestsUploadPostSomeNullableObject.from_dict(data)
277277

278278
return some_nullable_object_type_0
279-
except: # noqa: E722
279+
except (TypeError, ValueError, AttributeError, KeyError):
280280
pass
281281
return cast(BodyUploadFileTestsUploadPostSomeNullableObject | None, data)
282282

@@ -345,7 +345,7 @@ def _parse_some_array(data: object) -> list[AFormData] | None | Unset:
345345
some_array_type_0.append(some_array_type_0_item)
346346

347347
return some_array_type_0
348-
except: # noqa: E722
348+
except (TypeError, ValueError, AttributeError, KeyError):
349349
pass
350350
return cast(list[AFormData] | None | Unset, data)
351351

‎end_to_end_tests/golden-record/my_test_api_client/models/extended.py‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ def _parse_a_camel_date_time(data: object) -> datetime.date | datetime.datetime:
279279
a_camel_date_time_type_0 = isoparse(data)
280280

281281
return a_camel_date_time_type_0
282-
except: # noqa: E722
282+
except (TypeError, ValueError, AttributeError, KeyError):
283283
pass
284284
if not isinstance(data, str):
285285
raise TypeError()
@@ -300,7 +300,7 @@ def _parse_a_nullable_date(data: object) -> datetime.date | None:
300300
a_nullable_date_type_0 = isoparse(data).date()
301301

302302
return a_nullable_date_type_0
303-
except: # noqa: E722
303+
except (TypeError, ValueError, AttributeError, KeyError):
304304
pass
305305
return cast(datetime.date | None, data)
306306

@@ -317,7 +317,7 @@ def _parse_a_nullable_uuid(data: object) -> None | UUID:
317317
a_nullable_uuid_type_0 = UUID(data)
318318

319319
return a_nullable_uuid_type_0
320-
except: # noqa: E722
320+
except (TypeError, ValueError, AttributeError, KeyError):
321321
pass
322322
return cast(None | UUID, data)
323323

@@ -339,15 +339,15 @@ def _parse_one_of_models(data: object) -> Any | FreeFormModel | ModelWithUnionPr
339339
one_of_models_type_0 = FreeFormModel.from_dict(data)
340340

341341
return one_of_models_type_0
342-
except: # noqa: E722
342+
except (TypeError, ValueError, AttributeError, KeyError):
343343
pass
344344
try:
345345
if not isinstance(data, dict):
346346
raise TypeError()
347347
one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
348348

349349
return one_of_models_type_1
350-
except: # noqa: E722
350+
except (TypeError, ValueError, AttributeError, KeyError):
351351
pass
352352
return cast(Any | FreeFormModel | ModelWithUnionProperty, data)
353353

@@ -362,15 +362,15 @@ def _parse_nullable_one_of_models(data: object) -> FreeFormModel | ModelWithUnio
362362
nullable_one_of_models_type_0 = FreeFormModel.from_dict(data)
363363

364364
return nullable_one_of_models_type_0
365-
except: # noqa: E722
365+
except (TypeError, ValueError, AttributeError, KeyError):
366366
pass
367367
try:
368368
if not isinstance(data, dict):
369369
raise TypeError()
370370
nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
371371

372372
return nullable_one_of_models_type_1
373-
except: # noqa: E722
373+
except (TypeError, ValueError, AttributeError, KeyError):
374374
pass
375375
return cast(FreeFormModel | ModelWithUnionProperty | None, data)
376376

@@ -387,7 +387,7 @@ def _parse_nullable_model(data: object) -> ModelWithUnionProperty | None:
387387
nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
388388

389389
return nullable_model_type_1
390-
except: # noqa: E722
390+
except (TypeError, ValueError, AttributeError, KeyError):
391391
pass
392392
return cast(ModelWithUnionProperty | None, data)
393393

@@ -452,7 +452,7 @@ def _parse_not_required_one_of_models(data: object) -> FreeFormModel | ModelWith
452452
not_required_one_of_models_type_0 = FreeFormModel.from_dict(data)
453453

454454
return not_required_one_of_models_type_0
455-
except: # noqa: E722
455+
except (TypeError, ValueError, AttributeError, KeyError):
456456
pass
457457
if not isinstance(data, dict):
458458
raise TypeError()
@@ -475,15 +475,15 @@ def _parse_not_required_nullable_one_of_models(
475475
not_required_nullable_one_of_models_type_0 = FreeFormModel.from_dict(data)
476476

477477
return not_required_nullable_one_of_models_type_0
478-
except: # noqa: E722
478+
except (TypeError, ValueError, AttributeError, KeyError):
479479
pass
480480
try:
481481
if not isinstance(data, dict):
482482
raise TypeError()
483483
not_required_nullable_one_of_models_type_1 = ModelWithUnionProperty.from_dict(data)
484484

485485
return not_required_nullable_one_of_models_type_1
486-
except: # noqa: E722
486+
except (TypeError, ValueError, AttributeError, KeyError):
487487
pass
488488
return cast(FreeFormModel | ModelWithUnionProperty | None | str | Unset, data)
489489

@@ -509,7 +509,7 @@ def _parse_not_required_nullable_model(data: object) -> ModelWithUnionProperty |
509509
not_required_nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
510510

511511
return not_required_nullable_model_type_1
512-
except: # noqa: E722
512+
except (TypeError, ValueError, AttributeError, KeyError):
513513
pass
514514
return cast(ModelWithUnionProperty | None | Unset, data)
515515

0 commit comments

Comments
 (0)