Skip to content

Commit 79b6d5a

Browse files
kwzrdChrisLovering
authored andcommitted
Branding: handle repo errors in cog
If we fail to fetch an event, the whole branding sync will now be aborted. This will prevent situations where we fail to fetch the current event due to a 5xx error and the cog resorts to the fallback branding in the middle of an event. Error handling is moved to the cog. The repo abstraction will now propagate errors rather than silence them.
1 parent 53eec3e commit 79b6d5a

File tree

3 files changed

+37
-29
lines changed

3 files changed

+37
-29
lines changed

bot/errors.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def __init__(self, converter: Converter, original: Exception, infraction_arg: in
5959

6060

6161
class BrandingMisconfigurationError(RuntimeError):
62-
"""Raised by the Branding cog when a misconfigured event is encountered."""
62+
"""Raised by the Branding cog when branding misconfiguration is detected."""
6363

6464

6565

bot/exts/backend/branding/_cog.py

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -342,14 +342,14 @@ async def synchronise(self) -> tuple[bool, bool]:
342342
"""
343343
log.debug("Synchronise: fetching current event.")
344344

345-
current_event, available_events = await self.repository.get_current_event()
345+
try:
346+
current_event, available_events = await self.repository.get_current_event()
347+
except Exception:
348+
log.exception("Synchronisation aborted: failed to fetch events.")
349+
return False, False
346350

347351
await self.populate_cache_events(available_events)
348352

349-
if current_event is None:
350-
log.error("Failed to fetch event. Cannot synchronise!")
351-
return False, False
352-
353353
return await self.enter_event(current_event)
354354

355355
async def populate_cache_events(self, events: list[Event]) -> None:
@@ -433,10 +433,6 @@ async def daemon_main(self) -> None:
433433

434434
await self.populate_cache_events(available_events)
435435

436-
if new_event is None:
437-
log.warning("Daemon main: failed to get current event from branding repository, will do nothing.")
438-
return
439-
440436
if new_event.path != await self.cache_information.get("event_path"):
441437
log.debug("Daemon main: new event detected!")
442438
await self.enter_event(new_event)
@@ -596,8 +592,24 @@ async def branding_calendar_refresh_cmd(self, ctx: commands.Context) -> None:
596592
log.info("Performing command-requested event cache refresh.")
597593

598594
async with ctx.typing():
599-
available_events = await self.repository.get_events()
600-
await self.populate_cache_events(available_events)
595+
try:
596+
available_events = await self.repository.get_events()
597+
except Exception:
598+
log.exception("Refresh aborted: failed to fetch events.")
599+
resp = make_embed(
600+
"Refresh aborted",
601+
"Failed to fetch events. See log for details.",
602+
success=False,
603+
)
604+
else:
605+
await self.populate_cache_events(available_events)
606+
resp = make_embed(
607+
"Refresh successful",
608+
"The event calendar has been refreshed.",
609+
success=True,
610+
)
611+
612+
await ctx.send(embed=resp)
601613

602614
await ctx.invoke(self.branding_calendar_group)
603615

bot/exts/backend/branding/_repository.py

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -186,37 +186,34 @@ async def get_events(self) -> list[Event]:
186186
"""
187187
Discover available events in the branding repository.
188188
189-
Misconfigured events are skipped. May return an empty list in the catastrophic case.
189+
Propagate errors if an event fails to fetch or deserialize.
190190
"""
191191
log.debug("Discovering events in branding repository.")
192192

193-
try:
194-
event_directories = await self.fetch_directory("events", types=("dir",)) # Skip files.
195-
except Exception:
196-
log.exception("Failed to fetch 'events' directory.")
197-
return []
193+
event_directories = await self.fetch_directory("events", types=("dir",)) # Skip files.
198194

199195
instances: list[Event] = []
200196

201197
for event_directory in event_directories.values():
202-
log.trace(f"Attempting to construct event from directory: '{event_directory.path}'.")
203-
try:
204-
instance = await self.construct_event(event_directory)
205-
except Exception as exc:
206-
log.warning(f"Could not construct event '{event_directory.path}'.", exc_info=exc)
207-
else:
208-
instances.append(instance)
198+
log.trace(f"Reading event directory: '{event_directory.path}'.")
199+
instance = await self.construct_event(event_directory)
200+
instances.append(instance)
209201

210202
return instances
211203

212-
async def get_current_event(self) -> tuple[Event | None, list[Event]]:
204+
async def get_current_event(self) -> tuple[Event, list[Event]]:
213205
"""
214206
Get the currently active event, or the fallback event.
215207
216208
The second return value is a list of all available events. The caller may discard it, if not needed.
217209
Returning all events alongside the current one prevents having to query the API twice in some cases.
218210
219-
The current event may be None in the case that no event is active, and no fallback event is found.
211+
Raise an error in the following cases:
212+
* GitHub request fails
213+
* The branding repo contains an invalid event
214+
* No event is active and the fallback event is missing
215+
216+
Events are validated in the branding repo. The bot assumes that events are valid.
220217
"""
221218
utc_now = datetime.now(tz=UTC)
222219
log.debug(f"Finding active event for: {utc_now}.")
@@ -249,5 +246,4 @@ async def get_current_event(self) -> tuple[Event | None, list[Event]]:
249246
if event.meta.is_fallback:
250247
return event, available_events
251248

252-
log.warning("No event is currently active and no fallback event was found!")
253-
return None, available_events
249+
raise BrandingMisconfigurationError("No event is active and the fallback event is missing!")

0 commit comments

Comments
 (0)