Skip to content

Commit e3dc076

Browse files
tujtuj
authored
Merge pull request os2display#386 from os2display/feature/better-feed-exception-handling
Fixed feed errors resulting in long caching of empty arrays for getData calls
2 parents b6d844d + ad44858 commit e3dc076

12 files changed

Lines changed: 147 additions & 57 deletions

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ All notable changes to this project will be documented in this file.
2424
- Ensure the http client has a default time out setting. Make it configurable in env.
2525
- [#376](https://github.com/os2display/display-api-service/pull/376)
2626
- Add prod override for cache.app to use Redis in production.
27+
- [#386](https://github.com/os2display/display-api-service/pull/386)
28+
- Add better cache handling when getData throws errors.
2729

2830
## [2.6.1] - 2026-03-06
2931

‎src/Feed/BrndFeedType.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function getData(Feed $feed): array
7878

7979
$bookings = $this->apiClient->getInfomonitorBookingsDetails($feedSource, $sportCenterId);
8080

81-
$result['bookings'] = array_reduce($bookings, function (array $carry, array $booking): array {
81+
return array_reduce($bookings, function (array $carry, array $booking): array {
8282
$parsedBooking = $this->parseBrndBooking($booking);
8383

8484
// Validate that booking has required fields
@@ -90,11 +90,9 @@ public function getData(Feed $feed): array
9090
}, []);
9191
} catch (\Throwable $throwable) {
9292
$this->logger->error($throwable->getMessage());
93-
// Silently catch all exceptions and return empty result
94-
// $result is already initialized with empty bookings array
95-
}
9693

97-
return $result;
94+
throw $throwable;
95+
}
9896
}
9997

10098
private function parseBrndBooking(array $booking): array

‎src/Feed/CalendarApiFeedType.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ public function getData(Feed $feed): array
6767
$enabledModifiers = $configuration['enabledModifiers'] ?? [];
6868

6969
if (!isset($configuration['resources'])) {
70-
$this->logger->error('CalendarApiFeedType: Resources not set.');
71-
72-
return [];
70+
throw new \RuntimeException('CalendarApiFeedType: Resources not set.');
7371
}
7472

7573
$requestedResources = $configuration['resources'];
@@ -114,9 +112,9 @@ public function getData(Feed $feed): array
114112
'message' => $throwable->getMessage(),
115113
'exception' => $throwable,
116114
]);
117-
}
118115

119-
return [];
116+
throw $throwable;
117+
}
120118
}
121119

122120
public static function applyModifiersToEvents(array $events, array $eventModifiers, array $enabledModifiers): array

‎src/Feed/EventDatabaseApiFeedType.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function getData(Feed $feed): array
3838
$configuration = $feed->getConfiguration();
3939

4040
if (!isset($secrets['host'])) {
41-
return [];
41+
throw new \RuntimeException('EventDatabaseApiFeedType: Host secret is not set.');
4242
}
4343

4444
$host = $secrets['host'];
@@ -121,8 +121,14 @@ public function getData(Feed $feed): array
121121

122122
return [$eventOccurrence];
123123
}
124+
125+
throw new \RuntimeException('EventDatabaseApiFeedType: singleSelectedOccurrence is not set.');
126+
default:
127+
throw new \RuntimeException(sprintf('EventDatabaseApiFeedType: Unsupported posterType "%s".', $configuration['posterType']));
124128
}
125129
}
130+
131+
throw new \RuntimeException('EventDatabaseApiFeedType: posterType is not set.');
126132
} catch (\Throwable $throwable) {
127133
// If the content does not exist anymore, unpublished the slide.
128134
if ($throwable instanceof ClientException && Response::HTTP_NOT_FOUND == $throwable->getCode()) {
@@ -148,9 +154,9 @@ public function getData(Feed $feed): array
148154
'message' => $throwable->getMessage(),
149155
]);
150156
}
151-
}
152157

153-
return [];
158+
throw $throwable;
159+
}
154160
}
155161

156162
/**

‎src/Feed/EventDatabaseApiV2FeedType.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public function getData(Feed $feed): array
5757
}
5858

5959
if (!isset($configuration['posterType'])) {
60-
return [];
60+
throw new \RuntimeException('EventDatabaseApiV2FeedType: posterType is not set.');
6161
}
6262

6363
return match ($configuration['posterType']) {
@@ -95,9 +95,9 @@ public function getData(Feed $feed): array
9595
'exception' => $throwable,
9696
]);
9797
}
98-
}
9998

100-
return [];
99+
throw $throwable;
100+
}
101101
}
102102

103103
private function getSubscriptionPosterOutput(FeedSource $feedSource, array $configuration): array

‎src/Feed/KobaFeedType.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ public function getData(Feed $feed): array
3838
$configuration = $feed->getConfiguration();
3939

4040
if (!isset($secrets['kobaHost']) || !isset($secrets['kobaApiKey'])) {
41-
$this->logger->error('KobaFeedType: "Host" and "ApiKey" not configured.');
42-
43-
return [];
41+
throw new \RuntimeException('KobaFeedType: "Host" and "ApiKey" not configured.');
4442
}
4543

4644
$kobaHost = $secrets['kobaHost'];
@@ -50,9 +48,7 @@ public function getData(Feed $feed): array
5048
$rewriteBookedTitles = $configuration['rewriteBookedTitles'] ?? false;
5149

5250
if (!isset($configuration['resources'])) {
53-
$this->logger->error('KobaFeedType: Resources not set.');
54-
55-
return [];
51+
throw new \RuntimeException('KobaFeedType: Resources not set.');
5652
}
5753

5854
$resources = $configuration['resources'];
@@ -121,9 +117,9 @@ public function getData(Feed $feed): array
121117
'code' => $throwable->getCode(),
122118
'message' => $throwable->getMessage(),
123119
]);
124-
}
125120

126-
return [];
121+
throw $throwable;
122+
}
127123
}
128124

129125
/**

‎src/Feed/NotifiedFeedType.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ public function getData(Feed $feed): array
3232
try {
3333
$secrets = $feed->getFeedSource()?->getSecrets();
3434
if (!isset($secrets['token'])) {
35-
return [];
35+
throw new \RuntimeException('NotifiedFeedType: Token secret is not set.');
3636
}
3737

3838
$configuration = $feed->getConfiguration();
3939
if (!isset($configuration['feeds']) || 0 === count($configuration['feeds'])) {
40-
return [];
40+
throw new \RuntimeException('NotifiedFeedType: Feeds configuration is not set.');
4141
}
4242

4343
$slide = $feed->getSlide();
@@ -75,9 +75,9 @@ public function getData(Feed $feed): array
7575
'code' => $throwable->getCode(),
7676
'message' => $throwable->getMessage(),
7777
]);
78-
}
7978

80-
return [];
79+
throw $throwable;
80+
}
8181
}
8282

8383
/**

‎src/Feed/RssFeedType.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function getData(Feed $feed): array
4545
$url = $configuration['url'] ?? null;
4646

4747
if (!isset($url)) {
48-
return [];
48+
throw new \RuntimeException('RssFeedType: URL is not set.');
4949
}
5050

5151
$feedResult = $this->feedIo->read($url);
@@ -74,9 +74,9 @@ public function getData(Feed $feed): array
7474
return $result;
7575
} catch (\Throwable $throwable) {
7676
$this->logger->error($throwable->getCode().': '.$throwable->getMessage());
77-
}
7877

79-
return [];
78+
throw $throwable;
79+
}
8080
}
8181

8282
/**

‎src/Feed/SparkleIOFeedType.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ public function getData(Feed $feed): array
4141
try {
4242
$secrets = $feed->getFeedSource()?->getSecrets();
4343
if (!isset($secrets['baseUrl']) || !isset($secrets['clientId']) || !isset($secrets['clientSecret'])) {
44-
return [];
44+
throw new \RuntimeException('SparkleIOFeedType: Required secrets (baseUrl, clientId, clientSecret) are not set.');
4545
}
4646

4747
$configuration = $feed->getConfiguration();
4848
if (!isset($configuration['feeds']) || 0 === count($configuration['feeds'])) {
49-
return [];
49+
throw new \RuntimeException('SparkleIOFeedType: Feeds configuration is not set.');
5050
}
5151

5252
$baseUrl = $secrets['baseUrl'];
@@ -78,9 +78,9 @@ public function getData(Feed $feed): array
7878
'code' => $throwable->getCode(),
7979
'message' => $throwable->getMessage(),
8080
]);
81-
}
8281

83-
return [];
82+
throw $throwable;
83+
}
8484
}
8585

8686
/**

‎src/Service/FeedService.php‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
class FeedService
1717
{
18+
private const int ERROR_CACHE_TTL_SECONDS = 30;
19+
1820
public function __construct(
1921
private readonly iterable $feedTypes,
2022
private readonly CacheInterface $feedsCache,
@@ -103,13 +105,25 @@ public function getData(Feed $feed): ?array
103105
/** @var FeedTypeInterface $feedType */
104106
foreach ($this->feedTypes as $feedType) {
105107
if ($feedType::class === $feedTypeClassName) {
106-
return $this->feedsCache->get($feedId, function (ItemInterface $item) use ($feed, $feedType, $feedConfiguration) {
107-
if (isset($feedConfiguration['cache_expire'])) {
108-
$item->expiresAfter($feedConfiguration['cache_expire']);
109-
}
110-
111-
return $feedType->getData($feed);
112-
});
108+
try {
109+
return $this->feedsCache->get($feedId, function (ItemInterface $item) use ($feed, $feedType, $feedConfiguration) {
110+
if (isset($feedConfiguration['cache_expire'])) {
111+
$item->expiresAfter($feedConfiguration['cache_expire']);
112+
}
113+
114+
return $feedType->getData($feed);
115+
});
116+
} catch (\Throwable) {
117+
// Cache empty result with a short TTL to prevent stampeding
118+
// the failing service with repeated requests.
119+
$this->feedsCache->delete($feedId);
120+
121+
return $this->feedsCache->get($feedId, function (ItemInterface $item) {
122+
$item->expiresAfter(self::ERROR_CACHE_TTL_SECONDS);
123+
124+
return [];
125+
});
126+
}
113127
}
114128
}
115129

0 commit comments

Comments
 (0)