fix: batch A - crash bugs, a leak, and startup/edge fragility
CI / compile (pull_request) Successful in 10s
CI / unit (pull_request) Successful in 19s
CI / integration (pull_request) Successful in 10s
build / build (push) Failing after 46m40s
CI / compile (push) Successful in 20s
CI / unit (push) Successful in 19s
CI / integration (push) Successful in 16s

Seven confirmed defects from the code audit, each small and low-risk.

* ai_functions.get_random_cyclic_message: random.randint(0, len(CYCLIC_WORDS))
  is inclusive -> could return len -> IndexError. Now randrange(len) + guard on
  an empty CYCLIC_WORDS.
* librarian_commands.get_image_sadox: random.randrange(0, len(res)-1) never
  picked the last comic and raised ValueError('empty range') on a single file.
  Now randrange(len) + an empty-dir guard.
* ai_commands image generation: every DALL-E error branch replied but did not
  return, so control fell through to `if response:` with response unbound ->
  UnboundLocalError right after the friendly message. Each branch now returns;
  response is pre-initialised; and PermissionDeniedError no longer passes a
  (message, text) tuple as a single arg.
* search_bot DOI match: `item["DOI"] in data` was a substring test, so a DOI
  that is a prefix of a longer one (10.1/1 vs 10.1/12) produced a false 'exists'
  hit. Now matches the line's first whitespace token exactly, via an O(1) dict
  index built once per consumer (also removes the O(queried-DOIs) per-line scan
  - a real win for large databases).
* communication_subroutine.scan_incoming: matched records were never removed
  from awaiting_q, so it grew unbounded over uptime and a reused UUID could
  re-match a stale record. Matched records are now dropped after dispatch.
* communication_subroutine.id3: (resp.headers.get("icy-name") or "").title()
  guards against a stream that omits headers (was AttributeError on None,
  500-ing the /prepped_tracks "next" handler).
* betoniarka.scan_tracks: waits for the radio logs to exist instead of dying
  with FileNotFoundError on a fresh deploy (which silently killed the
  now-playing forwarder until a restart).

Verified: tests/unit/test_search_bot.py gains exact-match and trailing-metadata
cases; full unit job 43 passed. Remaining observations (image-gen stale
/home/pi fallback paths + dead FileNotFoundError-after-OSError branch; tailer
still vulnerable to mid-run log rotation; DOI-first-token assumption) noted for
follow-up - none are crashes on the normal path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit was merged in pull request #8.
This commit is contained in:
2026-07-31 23:01:27 +02:00
parent e1fca864d8
commit defc482a22
7 changed files with 90 additions and 22 deletions
+14 -3
View File
@@ -427,6 +427,11 @@ class Events(commands.Cog):
return return
message_content_lower = message_content_lower.replace("imaginuje sobie: ", "") message_content_lower = message_content_lower.replace("imaginuje sobie: ", "")
self.logger.debug("Wywolanie obrazka: %s", message_content_lower) self.logger.debug("Wywolanie obrazka: %s", message_content_lower)
# Every error branch below must RETURN: otherwise control falls
# through to `if response:` with `response` unbound (the call
# raised) -> UnboundLocalError, crashing the handler right after
# the friendly message was already sent.
response = None
try: try:
response = await OPENAICLIENT.images.generate( response = await OPENAICLIENT.images.generate(
model="dall-e-3", model="dall-e-3",
@@ -440,10 +445,12 @@ class Events(commands.Cog):
await discord_friendly_reply( await discord_friendly_reply(
message, f"*Kondziu patrzy na terminal, czeka, czeka, czeka,.... Jeszcze chwile czeka Przypierdala w niego pięścią....* Nie mogę się połączyć z Openai spróbuj od nowa. *Na ekranie pojawia się*: {e}" message, f"*Kondziu patrzy na terminal, czeka, czeka, czeka,.... Jeszcze chwile czeka Przypierdala w niego pięścią....* Nie mogę się połączyć z Openai spróbuj od nowa. *Na ekranie pojawia się*: {e}"
) )
return
except openai.APIConnectionError as e: except openai.APIConnectionError as e:
await discord_friendly_reply( await discord_friendly_reply(
message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Nie mogę się połączyć z Openai. *Na ekranie pojawia się*: {e}" message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Nie mogę się połączyć z Openai. *Na ekranie pojawia się*: {e}"
) )
return
except openai.BadRequestError as e: except openai.BadRequestError as e:
# Handle invalid request error, e.g. validate parameters or log # Handle invalid request error, e.g. validate parameters or log
if message.author.nick: if message.author.nick:
@@ -461,27 +468,31 @@ class Events(commands.Cog):
await discord_friendly_reply( await discord_friendly_reply(
message, f"Sorki, cenzura: {resp}. Jak chcesz to są kanały na nudle #sexy-foteczky i #kanal-do-fapania *Na ekranie pojawia się: {e}" message, f"Sorki, cenzura: {resp}. Jak chcesz to są kanały na nudle #sexy-foteczky i #kanal-do-fapania *Na ekranie pojawia się: {e}"
) )
return
except openai.AuthenticationError as e: except openai.AuthenticationError as e:
# Handle authentication error, e.g. check credentials or log # Handle authentication error, e.g. check credentials or log
await discord_friendly_reply( await discord_friendly_reply(
message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Wołaj szefa - coś się z hasłem zjebało. *Na terminalu pojawia się:* {e}" message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Wołaj szefa - coś się z hasłem zjebało. *Na terminalu pojawia się:* {e}"
) )
return
except openai.PermissionDeniedError as e: except openai.PermissionDeniedError as e:
# Handle permission error, e.g. check scope or log # Handle permission error, e.g. check scope or log
# (was accidentally passing a (message, text) TUPLE as one arg)
await discord_friendly_reply( await discord_friendly_reply(
( message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Wołaj szefa - coś się z uprawnieniami zjebało. *Na terminalu pojawia się:* {e}"
message, f"*Kondziu patrzy na terminal, chwile się zastanawia. Przypierdala w niego pięścią....* Wołaj szefa - coś się z uprawnieniami zjebało. *Na terminalu pojawia się:* {e}"
)
) )
return
except openai.RateLimitError as e: except openai.RateLimitError as e:
await discord_friendly_reply( await discord_friendly_reply(
message, f"*Kondziu patrzy na terminal* Wołaj szefa. Zapłacić rachunki za AI trzeba. Jak chcesz to się na #zebranie dorzuć. {e}" message, f"*Kondziu patrzy na terminal* Wołaj szefa. Zapłacić rachunki za AI trzeba. Jak chcesz to się na #zebranie dorzuć. {e}"
) )
return
except openai.APIError as e: except openai.APIError as e:
# Handle API error, e.g. retry or log # Handle API error, e.g. retry or log
await discord_friendly_reply( await discord_friendly_reply(
message, f"*Kondziu nurkuje za bar, terminal wybucha. Przed tobą ląduje pergamin zapisany pięknym gotykiem a na nim*: {e}" message, f"*Kondziu nurkuje za bar, terminal wybucha. Przed tobą ląduje pergamin zapisany pięknym gotykiem a na nim*: {e}"
) )
return
if response: if response:
self.logger.info(response) self.logger.info(response)
image_url = response.data[0].url image_url = response.data[0].url
+4 -2
View File
@@ -541,10 +541,12 @@ async def get_random_cyclic_message(client):
# trunk-ignore(bandit/B311) # trunk-ignore(bandit/B311)
ai_check = random.randint(0, 10) ai_check = random.randint(0, 10)
logger.info("Losowa wypowiedź") logger.info("Losowa wypowiedź")
if ai_check < 2: if ai_check < 2 and CYCLIC_WORDS:
logger.info("Predefiniowana") logger.info("Predefiniowana")
# randrange(n) is 0..n-1; randint(0, n) was inclusive and could return n
# -> list(...)[n] IndexError. Guarded on empty CYCLIC_WORDS above.
# trunk-ignore(bandit/B311) # trunk-ignore(bandit/B311)
messnum = random.randint(0, len(CYCLIC_WORDS)) messnum = random.randrange(len(CYCLIC_WORDS))
logger.debug(messnum) logger.debug(messnum)
logger.debug(len(CYCLIC_WORDS)) logger.debug(len(CYCLIC_WORDS))
mess_key = list(CYCLIC_WORDS.keys())[messnum] mess_key = list(CYCLIC_WORDS.keys())[messnum]
+14 -5
View File
@@ -235,14 +235,20 @@ def scan_incoming(stop_event: Optional[threading.Event] = None):
try: try:
answer = incoming_q.get(block=False) answer = incoming_q.get(block=False)
logger.info("DATA FOUND") logger.info("DATA FOUND")
record_stored = False # Collect matched records and drop them from awaiting_q afterwards -
# they used to stay forever (awaiting_q only ever grew), leaking
# memory over the bot's uptime and letting a reused UUID re-match a
# stale record.
matched = []
for record in awaiting_q: for record in awaiting_q:
if record.uuid in answer.keys(): if record.uuid in answer.keys():
record_stored = True
record.stop = True record.stop = True
record.entries = answer[record.uuid] record.entries = answer[record.uuid]
IN_COMM_Q.put(record) IN_COMM_Q.put(record)
if not record_stored: matched.append(record)
for record in matched:
awaiting_q.remove(record)
if not matched:
for key in answer.keys(): for key in answer.keys():
record = QueryControl("Orphaned", key, "Orphan", None) record = QueryControl("Orphaned", key, "Orphan", None)
record.stop = True record.stop = True
@@ -270,10 +276,13 @@ def id3(url: str) -> dict:
resp.read( resp.read(
metaint metaint
) # this isn't seekable so, arbitrarily read to the point we want ) # this isn't seekable so, arbitrarily read to the point we want
# Guard the headers: an Icecast stream that omits icy-name / icy-genre
# (e.g. while the radio is down) made `.title()` raise AttributeError on
# None, 500-ing the /prepped_tracks "next" handler that calls this.
tagdata = dict( tagdata = dict(
site_url=resp.headers.get("icy-url"), site_url=resp.headers.get("icy-url"),
name=resp.headers.get("icy-name").title(), name=(resp.headers.get("icy-name") or "").title(),
genre=resp.headers.get("icy-genre").title(), genre=(resp.headers.get("icy-genre") or "").title(),
title=get_stream_title(resp.read(255)), title=get_stream_title(resp.read(255)),
) )
return tagdata return tagdata
+6
View File
@@ -203,6 +203,12 @@ def wyszukaj(word_list, how_many, _logger=None, write_to=None):
# ---------------------------------------------------------------- tailer # ---------------------------------------------------------------- tailer
def scan_tracks(): def scan_tracks():
"""Tail the radio logs and forward play events to the bot.""" """Tail the radio logs and forward play events to the bot."""
# On a fresh deploy Liquidsoap may not have written its logs yet; wait for
# them instead of dying with FileNotFoundError, which used to silently kill
# the now-playing forwarder until the container was restarted.
while not (RADIOLOG_PATH.exists() and PERSISTENCE_PATH.exists()):
logger.info("Waiting for radio logs (%s, %s)...", RADIOLOG_PATH, PERSISTENCE_PATH)
time.sleep(5)
with open(RADIOLOG_PATH, "r", encoding=ENCODING) as log_file: with open(RADIOLOG_PATH, "r", encoding=ENCODING) as log_file:
log_file.seek(os.stat(RADIOLOG_PATH).st_size) log_file.seek(os.stat(RADIOLOG_PATH).st_size)
prev_size = os.stat(PERSISTENCE_PATH).st_size prev_size = os.stat(PERSISTENCE_PATH).st_size
+19 -11
View File
@@ -148,8 +148,11 @@ def consumer(in_q, control_q, doi, live_results, result_list, control_dict, expe
print(f"Consumer thread started: {no} no") print(f"Consumer thread started: {no} no")
empty_counter = 0 empty_counter = 0
alive_no = 0 alive_no = 0
# DOI -> result item, so a line is matched with one O(1) dict lookup instead
# of scanning every queried DOI. Items are shared with result_list, so
# setting exists here is seen by everyone.
doi_index = {item["DOI"]: item for item in result_list}
while True: while True:
done_check = True
try: try:
data = in_q.get(block=True, timeout = 1) data = in_q.get(block=True, timeout = 1)
if data is _sentinel: if data is _sentinel:
@@ -161,16 +164,21 @@ def consumer(in_q, control_q, doi, live_results, result_list, control_dict, expe
alive_no += 1 alive_no += 1
print(f"C{no}__{alive_no}\r", end="") print(f"C{no}__{alive_no}\r", end="")
for item in result_list: # Each DB line is a DOI (optionally followed by metadata). Match
if item["DOI"] in data and not item["exists"]: # the WHOLE first token exactly - the old `item["DOI"] in data`
print(f"HIT in {no} content {data[0]} line {data[1]} file {data[2]} {item['exists']}") # was a substring test, so a DOI that is a prefix of a longer one
_logger.info(data) # (10.1/1 vs 10.1/12) produced a false 'exists' hit.
_logger.info("HIT") parts = data.split()
item["exists"] = True line_doi = parts[0] if parts else ""
live_results.append(item) item = doi_index.get(line_doi)
done_check = done_check and item["exists"] if item is not None and not item["exists"]:
if done_check: print(f"HIT in {no}: {line_doi}")
control_q.put(_sentinel) _logger.info("HIT %s", line_doi)
item["exists"] = True
live_results.append(item)
# All found? Signal producers to stop early (rare -> cheap).
if all(it["exists"] for it in result_list):
control_q.put(_sentinel)
except Empty: except Empty:
empty_counter += 1 empty_counter += 1
time.sleep(1) time.sleep(1)
+6 -1
View File
@@ -52,8 +52,13 @@ class DataModule(commands.Cog):
# check if current path is a file # check if current path is a file
if os.path.isfile(os.path.join(DIR_PATH_SADOX, path)): if os.path.isfile(os.path.join(DIR_PATH_SADOX, path)):
res.append(path) res.append(path)
if not res:
await ctx.send("*Conjurer grzebie w pustej skrzyni* Nie ma dziś żadnych komiksów.")
return
# randrange(len) is 0..len-1; the old randrange(0, len-1) never picked
# the last file and raised ValueError('empty range') on a single file.
# trunk-ignore(bandit/B311) # trunk-ignore(bandit/B311)
filename = res[random.randrange(0, len(res) - 1)] filename = res[random.randrange(len(res))]
# select random page # select random page
file = open(DIR_PATH_SADOX + filename, "rb") file = open(DIR_PATH_SADOX + filename, "rb")
if True: if True:
+27
View File
@@ -94,6 +94,33 @@ def test_survives_invalid_utf8_byte_and_still_finds_later_doi(tmp_path, monkeypa
assert hit, "DOI after the bad byte was not found - the file was aborted mid-read" assert hit, "DOI after the bad byte was not found - the file was aborted mid-read"
def test_doi_match_is_exact_not_substring(tmp_path, monkeypatch):
# A DB line "10.1/12" must NOT satisfy a search for "10.1/1" (the old
# `doi in line` substring test did). The exact DOI must still be found.
monkeypatch.setattr(search_bot, "DATABASE_PATH", str(tmp_path) + "/")
(tmp_path / "0_chunk.txt").write_text(
"10.1/12\n10.1/1\n10.2/999\n", encoding="utf-8"
)
finished, result = _run_bounded([("10.1/1", "DATA"), ("10.9/absent", "DATA")])
assert finished
by_doi = {r["DOI"]: r["exists"] for r in result}
assert by_doi["10.1/1"] is True # exact line present -> found
assert by_doi["10.9/absent"] is False
def test_doi_match_handles_line_with_trailing_metadata(tmp_path, monkeypatch):
# Lines of the form "<DOI>\t<metadata>" still match on the first token.
monkeypatch.setattr(search_bot, "DATABASE_PATH", str(tmp_path) + "/")
(tmp_path / "0_chunk.txt").write_text("10.5/abc\tsome title here\n", encoding="utf-8")
finished, result = _run_bounded([("10.5/abc", "DATA")])
assert finished
assert result[0]["exists"] is True
def test_discover_chunk_files_sorted_numerically(tmp_path, monkeypatch): def test_discover_chunk_files_sorted_numerically(tmp_path, monkeypatch):
monkeypatch.setattr(search_bot, "DATABASE_PATH", str(tmp_path) + "/") monkeypatch.setattr(search_bot, "DATABASE_PATH", str(tmp_path) + "/")
for n in (0, 2, 10, 1): for n in (0, 2, 10, 1):