Skip to content

Commit 9cec69a

Browse files
authored
Fix IndexError: list index out of range in resampler (#1117)
When awaiting the resamplers to finish resampling, we need to make a copy of the resamplers dict, because new resamplers could be added or removed while we are awaiting, which would cause the results to be out of sync. Here is a traceback of an instance of this bug: ``` 2024-11-25 12:45:26,052,52 WARNING [_broadcast.py:416] Broadcast receiver [Broadcast:GrpcStreamBroadcaster-raw-component-data-2006:_Receiver] is full. Oldest message was dropped. 2024-11-25 12:45:26,052,52 ERROR [_resampling.py:195] The resample() function got an unexpected error, restarting... Traceback (most recent call last): File "/home/marenz/frequenz/sdk/src/frequenz/sdk/microgrid/_resampling.py", line 179, in _log_resampling_task_error resampling_task.result() File "/home/marenz/frequenz/sdk/src/frequenz/sdk/timeseries/_resampling.py", line 509, in resample { File "/home/marenz/frequenz/sdk/src/frequenz/sdk/timeseries/_resampling.py", line 514, in <dictcomp> if isinstance(results[i], (Exception, asyncio.CancelledError)) ~~~~~~~^^^ IndexError: list index out of range ```
2 parents dd7c65f + 86430a7 commit 9cec69a

File tree

2 files changed

+25
-16
lines changed

2 files changed

+25
-16
lines changed

RELEASE_NOTES.md

+13-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
# Frequenz Python SDK Release Notes
22

3+
## Summary
4+
5+
<!-- Here goes a general summary of what this release is about -->
6+
7+
## Upgrading
8+
9+
<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
10+
11+
## New Features
12+
13+
<!-- Here goes the main new features and examples or instructions on how to use them -->
14+
315
## Bug Fixes
416

5-
* Fix bug with `LoggingConfigUpdater` not updating root logger level.
6-
* The `frequenz-quantities` dependency requirement was widened to allow any v1.x version (it was pinned to `1.0.0rc3` before).
17+
- Fix a bug in the resampler that could end up with an *IndexError: list index out of range* exception when a new resampler was added while awaiting the existing resampler to finish resampling.

src/frequenz/sdk/timeseries/_resampling.py

+12-14
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from collections.abc import AsyncIterator, Callable, Coroutine, Sequence
1515
from dataclasses import dataclass
1616
from datetime import datetime, timedelta, timezone
17-
from typing import cast
1817

1918
from frequenz.channels.timer import Timer, TriggerAllMissed, _to_microseconds
2019
from frequenz.quantities import Quantity
@@ -495,25 +494,24 @@ async def resample(self, *, one_shot: bool = False) -> None:
495494
self._config.resampling_period,
496495
)
497496

497+
# We need to make a copy here because we need to match the results to the
498+
# current resamplers, and since we await here, new resamplers could be added
499+
# or removed from the dict while we awaiting the resampling, which would
500+
# cause the results to be out of sync.
501+
resampler_sources = list(self._resamplers)
498502
results = await asyncio.gather(
499503
*[r.resample(self._window_end) for r in self._resamplers.values()],
500504
return_exceptions=True,
501505
)
502506

503507
self._window_end += self._config.resampling_period
504-
# We need the cast because mypy is not able to infer that this can only
505-
# contain Exception | CancelledError because of the condition in the list
506-
# comprehension below.
507-
exceptions = cast(
508-
dict[Source, Exception | asyncio.CancelledError],
509-
{
510-
source: results[i]
511-
for i, source in enumerate(self._resamplers)
512-
# CancelledError inherits from BaseException, but we don't want
513-
# to catch *all* BaseExceptions here.
514-
if isinstance(results[i], (Exception, asyncio.CancelledError))
515-
},
516-
)
508+
exceptions = {
509+
source: result
510+
for source, result in zip(resampler_sources, results)
511+
# CancelledError inherits from BaseException, but we don't want
512+
# to catch *all* BaseExceptions here.
513+
if isinstance(result, (Exception, asyncio.CancelledError))
514+
}
517515
if exceptions:
518516
raise ResamplingError(exceptions)
519517
if one_shot:

0 commit comments

Comments
 (0)