Skip to content

Commit 99add43

Browse files
roberto-buttiroberto-butti
committed
Refactor exception handling: Bubble up the exceptions
Previously, the exception was caught and handled locally. Now, the exception is propagated upwards to allow the caller to handle it appropriately. This improves error transparency and aligns with best practices.
1 parent 5509c41 commit 99add43

13 files changed

+137
-63
lines changed

‎README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,3 +835,12 @@ Feel free to open issues or submit pull requests to improve the package.
835835
## License
836836

837837
This SDK is licensed under the MIT License. See the LICENSE file for details.
838+
839+
## TODO List
840+
- [ ] removing ManagementApiClient::xxxApi() for using new xxxApi($client)
841+
- [ ] add validation in StoryApi for the content section in creating and updating story
842+
- [ ] bubble up the Symfony HTTP Client exception
843+
- [ ] create the specific Responses for each endpoint (like SpaceResponse and SpacesResponse)
844+
- [ ] define LocalizedPath class to allow to handle the localized path in setting the Story for creating and update the story
845+
- [ ] add the set method for localized field (for example set("heading", $value, "de")) for setting the `heading__i18n__de` field
846+

‎src/Endpoints/SpaceApi.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Storyblok\ManagementApi\Response\SpacesResponse;
1212
use Storyblok\ManagementApi\Response\StoryblokResponse;
1313
use Storyblok\ManagementApi\Response\StoryblokResponseInterface;
14+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1415

1516
class SpaceApi extends EndpointBase
1617
{
@@ -23,12 +24,16 @@ public function all(): SpacesResponse
2324
return new SpacesResponse($httpResponse, SpacesData::class);
2425
}
2526

27+
/**
28+
* @throws TransportExceptionInterface
29+
*/
2630
public function get(string $spaceId): SpaceResponse
2731
{
2832
$httpResponse = $this->makeHttpRequest(
2933
"GET",
3034
'/v1/spaces/' . $spaceId,
3135
);
36+
3237
return new SpaceResponse($httpResponse);
3338
}
3439

‎src/Endpoints/StoryApi.php

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Storyblok\ManagementApi\QueryParameters\PaginationParams;
1616
use Storyblok\ManagementApi\QueryParameters\StoriesParams;
1717
use Storyblok\ManagementApi\Response\StoryblokResponseInterface;
18+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1819

1920
/**
2021
* StoryApi handles all story-related operations in the Storyblok Management API
@@ -94,6 +95,7 @@ public function get(string $storyId): StoryblokResponseInterface
9495
*
9596
* @throws InvalidStoryDataException
9697
* @throws StoryblokApiException
98+
* @throws TransportExceptionInterface
9799
*/
98100
public function create(StoryData $storyData): StoryblokResponseInterface
99101
{
@@ -115,42 +117,37 @@ public function create(StoryData $storyData): StoryblokResponseInterface
115117
dataClass: StoryData::class,
116118
);
117119

118-
if ($response->isOk()) {
119-
$this->logger->info('Story created successfully', [
120-
'story_name' => $storyData->name(),
121-
]);
122-
return $response;
123-
}
124-
125-
$this->logger->error('Failed to create story', [
126-
'status_code' => $response->getResponseStatusCode(),
127-
'error_message' => $response->getErrorMessage(),
120+
$this->logger->info('Story created successfully', [
128121
'story_name' => $storyData->name(),
129122
]);
123+
return $response;
130124

131-
throw new StoryblokApiException(
132-
sprintf(
133-
'Failed to create story: %s (Status code: %d)',
134-
$response->getErrorMessage(),
135-
$response->getResponseStatusCode(),
136-
),
137-
$response->getResponseStatusCode(),
138-
);
139125
} catch (\Exception $exception) {
140126
if ($exception instanceof StoryblokApiException) {
127+
$this->logger->error('Failed to create story', [
128+
'status_code' => $exception->getCode(),
129+
'error_message' => $exception->getMessage(),
130+
'story_name' => $storyData->name(),
131+
]);
141132
throw $exception;
142133
}
143134

144135
$this->logger->error('Unexpected error while creating story', [
145136
'error' => $exception->getMessage(),
146137
'story_name' => $storyData->name(),
147138
]);
139+
throw $exception;
140+
/*
141+
new StoryblokApiException(
142+
sprintf(
143+
'Failed to create story: %s (Status code: %d) Error: %s',
144+
$storyData->name(),
145+
$exception->getCode(),
146+
$exception->getMessage(),
147+
),
148+
$exception->getCode(),
149+
);*/
148150

149-
throw new StoryblokApiException(
150-
'Failed to create story: ' . $exception->getMessage(),
151-
0,
152-
$exception,
153-
);
154151
}
155152
}
156153

‎src/Endpoints/StoryBulkApi.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function createStories(array $stories): \Generator
107107
yield $response->data();
108108
$retryCount = 0;
109109
break;
110-
} catch (StoryblokApiException $e) {
110+
} catch (\Exception $e) {
111111
if ($e->getCode() === self::RATE_LIMIT_STATUS_CODE) {
112112
if ($retryCount >= self::MAX_RETRIES) {
113113
$this->logger->error('Max retries reached while creating story', [

‎src/Endpoints/UserApi.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,20 @@
66

77
use Storyblok\ManagementApi\Data\UserData;
88
use Storyblok\ManagementApi\Response\StoryblokResponseInterface;
9+
use Storyblok\ManagementApi\Response\UserResponse;
10+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
911

1012
class UserApi extends EndpointBase
1113
{
12-
public function me(): StoryblokResponseInterface
14+
/**
15+
* @throws TransportExceptionInterface
16+
*/
17+
public function me(): UserResponse
1318
{
14-
return $this->makeRequest(
19+
$httpResponse = $this->makeHttpRequest(
1520
"GET",
16-
'/v1/users/me',
17-
dataClass: UserData::class,
21+
'/v1/users/me'
1822
);
23+
return new UserResponse($httpResponse);
1924
}
2025
}

‎src/ManagementApiClient.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ public function httpAssetClient(): HttpClientInterface
9292
return $this->httpAssetClient;
9393
}
9494

95-
public function spaceApi(): SpaceApi
96-
{
97-
return new SpaceApi($this);
98-
}
99-
10095
public function storyApi(string|int $spaceId, ?LoggerInterface $logger = null): StoryApi
10196
{
10297
return new StoryApi(
@@ -115,11 +110,6 @@ public function storyBulkApi(string|int $spaceId, ?LoggerInterface $logger = nul
115110
);
116111
}
117112

118-
public function userApi(): UserApi
119-
{
120-
return new UserApi($this);
121-
}
122-
123113
public function assetApi(string|int $spaceId): AssetApi
124114
{
125115
return new AssetApi($this, $spaceId);

‎src/Response/StoryblokResponse.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,6 @@ public function asJson(): string|false
144144
public function toArray(): array
145145
{
146146

147-
return $this->response->toArray(false);
147+
return $this->response->toArray();
148148
}
149149
}

‎src/Response/UserResponse.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Storyblok\ManagementApi\Response;
6+
7+
use Storyblok\ManagementApi\Data\UserData;
8+
use Storyblok\ManagementApi\Exceptions\StoryblokFormatException;
9+
use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
10+
use Symfony\Contracts\HttpClient\Exception\DecodingExceptionInterface;
11+
use Symfony\Contracts\HttpClient\Exception\RedirectionExceptionInterface;
12+
use Symfony\Contracts\HttpClient\Exception\ServerExceptionInterface;
13+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
14+
15+
class UserResponse extends StoryblokResponse implements StoryblokResponseInterface
16+
{
17+
/**
18+
* @throws TransportExceptionInterface
19+
* @throws ServerExceptionInterface
20+
* @throws RedirectionExceptionInterface
21+
* @throws DecodingExceptionInterface
22+
* @throws StoryblokFormatException
23+
* @throws ClientExceptionInterface
24+
*/
25+
#[\Override]
26+
public function data(): UserData
27+
{
28+
$key = "user";
29+
$array = $this->toArray();
30+
if (array_key_exists($key, $array)) {
31+
return new UserData($array[$key]);
32+
}
33+
34+
throw new StoryblokFormatException(sprintf("Expected '%s' in the response.", $key));
35+
}
36+
}

‎tests/Feature/AssetTest.php

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Storyblok\ManagementApi\QueryParameters\PaginationParams;
1010
use Symfony\Component\HttpClient\MockHttpClient;
1111
use Symfony\Component\HttpClient\Response\JsonMockResponse;
12+
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
1213

1314
test('Testing One asset, AssetData', function (): void {
1415
$responses = [
@@ -32,9 +33,15 @@
3233
->and($storyblokData->createdAt())->toBe('2025-01-18')
3334
->and($storyblokData->updatedAt())->toBe('2025-01-19');
3435

35-
$storyblokResponse = $assetApi->get("111notexists");
36-
expect($storyblokResponse->getResponseStatusCode())->toBe(404) ;
37-
expect($storyblokResponse->asJson())->toBe('["This record could not be found"]');
36+
expect(
37+
function () use ($assetApi, $storyblokData): void {
38+
$storyblokResponse = $assetApi->get("111notexists");
39+
}
40+
)->toThrow(Exception::class, 'HTTP 404 returned for "https://example.com/v1/spaces/222/assets/111notexists');
41+
42+
//$storyblokResponse = $assetApi->get("111notexists");
43+
//expect($storyblokResponse->getResponseStatusCode())->toBe(404) ;
44+
//expect($storyblokResponse->asJson())->toBe('["This record could not be found"]');
3845

3946
});
4047

@@ -61,11 +68,18 @@
6168
expect($storyblokResponse->total())->toBe(2);
6269
expect($storyblokResponse->perPage())->toBe(25);
6370

64-
$storyblokResponse = $assetApi->page(page: new \Storyblok\ManagementApi\QueryParameters\PaginationParams(page: 100000));
65-
expect($storyblokResponse->getResponseStatusCode())->toBe(404) ;
66-
expect($storyblokResponse->asJson())->toBe('["This record could not be found"]');
67-
expect($storyblokResponse->isOk())->toBeFalse() ;
68-
expect($storyblokResponse->getErrorMessage())->toStartWith("404 - Not Found.") ;
71+
expect(function () use ($assetApi, $storyblokData): void {
72+
$storyblokResponse = $assetApi->page(page: new \Storyblok\ManagementApi\QueryParameters\PaginationParams(page: 100000));
73+
74+
})->toThrow(
75+
\Symfony\Component\HttpClient\Exception\ClientException::class,
76+
'HTTP 404 returned for "https://example.com/v1/spaces/222/assets?page=100000&per_page=25'
77+
);
78+
79+
//expect($storyblokResponse->getResponseStatusCode())->toBe(404) ;
80+
//expect($storyblokResponse->asJson())->toBe('["This record could not be found"]');
81+
//expect($storyblokResponse->isOk())->toBeFalse() ;
82+
//expect($storyblokResponse->getErrorMessage())->toStartWith("404 - Not Found.") ;
6983

7084
$assetsData = AssetsData::make([]);
7185
expect($assetsData)->toBeInstanceOf(AssetsData::class);

‎tests/Feature/ManagementApiTest.php

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,21 @@
4141
expect($tag->get("id"))->toBeNumeric()->toBeGreaterThan(1000);
4242
}
4343

44-
$response = $managementApi->get(
45-
sprintf('spaces/%s/internal_tags', $spaceId),
46-
[
47-
"by_object_type" => "asset",
48-
"search" => "somethingnotexists",
49-
]
44+
expect(function () use ($managementApi, $spaceId): void {
45+
$response = $managementApi->get(
46+
sprintf('spaces/%s/internal_tags', $spaceId),
47+
[
48+
"by_object_type" => "asset",
49+
"search" => "somethingnotexists",
50+
]
51+
);
52+
53+
})->toThrow(
54+
\Symfony\Component\HttpClient\Exception\ClientException::class,
55+
'HTTP 404 returned for "https://mapi.storyblok.com/v1/spaces/321388/internal_tags?by_object_type=asset&search=somethingnotexists'
5056
);
51-
expect($response->getResponseStatusCode())->toBe(404) ;
52-
expect($response->asJson())->toBe('["This record could not be found"]');
57+
//expect($response->getResponseStatusCode())->toBe(404) ;
58+
//expect($response->asJson())->toBe('["This record could not be found"]');
5359

5460
});
5561

0 commit comments

Comments
 (0)