Skip to content

Commit 601c582

Browse files
committed
fix(worker): surface EXPIRED_LINK / INVALID_LINK instead of UNKNOWN_ERROR
Two bugs were combining to mask real failures: 1. download_file treated any non-200 HEAD as INVALID_LINK, including the 4xx response that click.discord.com → GCS gives once the signed URL has expired. The user's link looked fine when they pasted it; it just timed out before the worker got to it. 2. process_package had `expected = ('EXPIRED_LINK')` — a string, not a tuple, because the parens are grouping rather than building a tuple. The substring check (`if expected not in current`) only recognized EXPIRED_LINK; INVALID_LINK and any future codes got relabeled as UNKNOWN_ERROR. Net effect from the user's POV: they'd retry an expired link and see 'UNKNOWN_ERROR' three times before realizing they needed a fresh export. Now: - HEAD 4xx → EXPIRED_LINK (the common case for stale links). - HEAD non-200 / wrong content-type → INVALID_LINK. - Anything else → UNKNOWN_ERROR with the full traceback preserved. - The expected-codes list is a real tuple containing both known codes.
1 parent de1c31d commit 601c582

1 file changed

Lines changed: 17 additions & 2 deletions

File tree

src/tasks.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ def download_file(package_status_id, package_id, link, session):
122122
r = requests.head(link, allow_redirects=True)
123123
print(r.status_code)
124124
print(r.headers)
125+
# Discord download URLs are signed and short-lived: click.discord.com
126+
# 302s to a Google Cloud Storage signed URL, which returns 4xx (most
127+
# commonly 400) once the signature TTL is up. Surface that as
128+
# EXPIRED_LINK so the user sees a useful message instead of the
129+
# generic UNKNOWN_ERROR fallback.
130+
if 400 <= r.status_code < 500:
131+
print(f'Link is expired (upstream returned {r.status_code}).')
132+
raise Exception('EXPIRED_LINK')
125133
if r.status_code != 200 or 'content-type' not in r.headers or 'application/octet-stream' not in r.headers['content-type']:
126134
print('The link does not point to a valid file.')
127135
raise Exception('INVALID_LINK')
@@ -973,10 +981,17 @@ def process_package(package_status_id, package_id, link, worker_name='regular_pr
973981
tb = ''.join(traceback.format_exception(type(e), e, e.__traceback__))
974982
print(f'process_package failed for {package_id}:\n{tb}')
975983

976-
expected = ('EXPIRED_LINK')
984+
# Worker raises these as known error codes. Anything else gets
985+
# relabeled UNKNOWN_ERROR with the full traceback preserved for
986+
# CloudWatch debugging.
987+
# NOTE: this used to be `expected = ('EXPIRED_LINK')` which is
988+
# just the string 'EXPIRED_LINK' (Python tuples need a comma);
989+
# the `in` check then accidentally did substring match on a
990+
# single code, hiding INVALID_LINK behind UNKNOWN_ERROR.
991+
EXPECTED_ERROR_CODES = ('EXPIRED_LINK', 'INVALID_LINK')
977992
current = str(e)
978993
e_traceback = None
979-
if expected not in current:
994+
if current not in EXPECTED_ERROR_CODES:
980995
current = 'UNKNOWN_ERROR'
981996
e_traceback = tb
982997
session.query(PackageProcessStatus).filter(PackageProcessStatus.id == package_status_id).update({

0 commit comments

Comments
 (0)