From 60aa580dd46d2fbcfa3d9727fa77e01504affa42 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sat, 4 Apr 2026 22:18:52 -0400 Subject: [PATCH 1/2] Make multiupload/finish idempotent to prevent cascade of 500 errors on retry When a client calls multiupload/finish and the reverse proxy times out (504) before the response reaches the client, the client retries. Each retry would create a duplicate UploadedFile record (causing NotUniqueError) or fail reading already-deleted chunks (FileNotFoundError), both surfacing as 500 errors. This was observed in production as 17 rapid-fire 500s. Check file.done at the top of finish_chunked_upload and return success immediately if the upload was already finalized, avoiding redundant reassembly and the resulting errors. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/upload.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/upload.py b/app/controllers/upload.py index 36dd9c6..a805044 100644 --- a/app/controllers/upload.py +++ b/app/controllers/upload.py @@ -144,6 +144,9 @@ def finish_chunked_upload(): if not file: abort(404) + if file.done: + return jsonify(result=True) + record = UploadedFile(expires=time.time() + 60 * 60, checksum=request.form['checksum'], content_checksum=request.form.get('content_checksum'), From 89182be96e2dddc17df0087c759bff3004e5f972 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Tue, 7 Apr 2026 21:07:35 -0400 Subject: [PATCH 2/2] Fix missing UploadedFile causing silent release failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The multiupload/finish idempotency path returned success without verifying the UploadedFile DB record still exists. If the record was lost (e.g. expired) after the upload completed, the release endpoint would reject the mod with "archive missing" — but clients like KNet never saw the error because create_release() returned HTTP 200 for all error responses. Two fixes: - multiupload/finish now checks for the UploadedFile record when ChunkedUpload.done is True, and re-creates it from the file on disk if missing. If the file is also gone, it resets done=False so the client can retry. - create_release() error responses now return proper HTTP status codes (400/403/500) instead of 200. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/mod.py | 10 +++++----- app/controllers/upload.py | 27 ++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/controllers/mod.py b/app/controllers/mod.py index 9015eab..1da72f9 100644 --- a/app/controllers/mod.py +++ b/app/controllers/mod.py @@ -326,7 +326,7 @@ def create_release(): meta, mod, release, user, error = _do_preflight(save=True) if error: app.logger.error('Rejecting release for mod %s due to %s', mod.mid, error) - return jsonify(result=False, reason=error) + return jsonify(result=False, reason=error), 400 files = [] for pmeta in meta['packages']: @@ -350,7 +350,7 @@ def create_release(): for ameta in pmeta['files']: if ameta['checksum'][0] != 'sha256': app.logger.error('Unsupported checksum for mod %s found: %s', mod.mid, ameta['checksum'][0]) - return jsonify(result=False, reason='unsupported archive checksum') + return jsonify(result=False, reason='unsupported archive checksum'), 400 archive = ModArchive(filename=ameta['filename'], dest=ameta['dest'], @@ -360,14 +360,14 @@ def create_release(): if 'urls' in ameta: if user.username not in app.config['URLS_FOR']: app.logger.error('Found URLs in upload for mod %s from %s', mod.mid, user.username) - return jsonify(result=False, reason='urls unauthorized') + return jsonify(result=False, reason='urls unauthorized'), 403 archive.urls = ameta['urls'] else: file = UploadedFile.objects(checksum=archive.checksum).first() if not file: app.logger.error('Missing file %s (%s) for mod %s', ameta['filename'], archive.checksum, mod.mid) - return jsonify(result=False, reason='archive missing', archive=ameta['filename']) + return jsonify(result=False, reason='archive missing', archive=ameta['filename']), 400 #if file.duplicate_of: # orig = UploadedFile.objects(checksum=file.duplicate_of).first() @@ -407,7 +407,7 @@ def create_release(): release.save() except ValidationError as exc: app.logger.error('Failed to save release for mod %s due to %s', mod.mid, exc) - return jsonify(result=False, reason=str(exc)) + return jsonify(result=False, reason=str(exc)), 500 for file in files: if not file.mod: diff --git a/app/controllers/upload.py b/app/controllers/upload.py index a805044..967b2f0 100644 --- a/app/controllers/upload.py +++ b/app/controllers/upload.py @@ -145,6 +145,28 @@ def finish_chunked_upload(): abort(404) if file.done: + record = UploadedFile.objects(checksum=request.form['checksum']).first() + if record: + return jsonify(result=True) + + # Upload was marked done but UploadedFile record is missing. + # The file should already exist in permanent storage from the previous run. + app.logger.warning('Upload %s marked done but UploadedFile record for %s is missing, re-creating', file.id, request.form['checksum']) + record = UploadedFile(expires=-1, + checksum=request.form['checksum'], + content_checksum=request.form.get('content_checksum'), + vp_checksum=request.form.get('vp_checksum'), + filesize=file.filesize) + record.gen_filename() + + full_path = os.path.join(app.config['FILE_STORAGE'], os.path.normpath(record.filename)) + if not os.path.isfile(full_path): + app.logger.error('Upload %s marked done but assembled file is also missing at %s', file.id, full_path) + file.done = False + file.save() + return jsonify(result=False, reason='upload lost, please retry'), 500 + + record.save() return jsonify(result=True) record = UploadedFile(expires=time.time() + 60 * 60, @@ -162,7 +184,6 @@ def finish_chunked_upload(): os.makedirs(os.path.dirname(full_path), exist_ok=True) h = hashlib.new('sha256') - failed = False try: with open(full_path, 'wb') as hdl: for num in range(file.total_parts): @@ -173,7 +194,7 @@ def finish_chunked_upload(): data = chunk.read(1024 * 1024) if not data: break - + h.update(data) hdl.write(data) except Exception: @@ -186,7 +207,7 @@ def finish_chunked_upload(): os.unlink(full_path) record.delete() return jsonify(result=False, reason='checksum fail') - + record.make_permanent() file.done = True file.save()