Skip to content

fix(toolkit): keep every failure when upload initiation and fallback both fail (PLAT-5848) - #1214

Merged
buraksenn merged 5 commits into
mainfrom
buraksenn/plat-5848-upload-fallback-errors
Oct 13, 2026
Merged

buraksenn merged 5 commits into
mainfrom
buraksenn/plat-5848-upload-fallback-errors

Conversation

@buraksenn

@buraksenn buraksenn commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Why

An app saw Error initiating upload. Status 400: Bad Request and lost 32 requests
(PLAT-5848). That message was useless twice over:

  • the SDK dropped the server's response body, so {"detail":"Invalid storage type"} never reached the app
  • it was the error of the GCS fallback, which most accounts aren't allowed to use
    (LEGACY_STORAGE_UPLOAD_ALLOWED_USERS); the CDN failure that triggered the fallback
    was only in runner stdout

What

  • providers/fal.py: initiate errors now include the requested storage type and up
    to 4 KiB of the server response, chained from the original HTTPError. Applied to
    all six initiation paths (single/multipart, GCS/V3/internal V3).
  • file.py _try_with_fallback: when the primary and every fallback fail, raise
    FileUploadFallbackException (plain subclass of FileUploadException) whose message
    lists each attempt's error. With no fallback configured the original exception is
    re-raised untouched. Exceptions with falls_back = False are re-raised immediately, as on main.
  • Fallback no longer dumps the failed attempt's traceback; one line per attempt
    (Failed to save to repository fal_v3: <Type>: <msg>, falling back to fal). The final
    exception chains the last cause.

Before:

Error initiating upload. Status 400: Bad Request

After:

Failed to save to all 2 repositories. fal_v3: FileUploadException: Error initiating upload. Status 500: Internal Server Error. Storage type: fal-cdn-v3; fal: FileUploadException: Error initiating upload. Status 400: Bad Request. Storage type: gcs. Server response: {"detail":"Invalid storage type"}

No retry-policy change: 400 stays non-retryable.

Tests

Unit tests for all six initiation paths (JSON / plain / invalid UTF-8 / empty /
truncated bodies, exception chaining, attempt counts) and for the fallback wrapper
(both fail, mixed exception types, no-fallback keeps original exception, pickle
round-trip, one print line per fallback). Verified end to end with fal run and a
forced failure on both attempts.


Note

Medium Risk
Changes exception types/messages on the hot file-upload path when fallbacks exhaust, but behavior for falls_back=False and single-repo failures is preserved; existing FileUploadException handlers still apply.

Overview
Improves file upload error reporting when CDN initiation fails and optional repository fallbacks also fail, so apps see the primary failure and server details instead of only the last fallback error.

Initiate-upload errors (providers/fal.py): a shared _initiate_upload_error helper now adds the storage type, up to 4 KiB of the HTTP error body (with truncation and safe UTF-8 handling), and chains the original HTTPError. All GCS/V3 initiation paths use it; retry behavior for 4xx vs 5xx is unchanged.

Repository fallback (file.py): when every attempt fails, raises new FileUploadFallbackException (still a FileUploadException) with a message listing each repo’s error in order; single-repo / no-fallback still re-raises the original exception. Fallback logging drops full tracebacks in favor of one line per failed attempt (without leaking credentials from repo objects).

Reviewed by Cursor Bugbot for commit 063905a. Bugbot is set up for automated code reviews on this repo. Configure here.

…both fail (PLAT-5848)

Upload initiation errors dropped the server response body, and when the
GCS fallback was denied its 400 hid the CDN failure that triggered it.

- providers/fal.py: initiate errors include the requested storage type
  and up to 4 KiB of the server response, chained from the HTTPError.
- file.py: when the primary and every fallback fail, raise
  FileUploadFallbackException (a FileUploadException) listing each
  attempt's error. Without a fallback the original exception is
  re-raised untouched.
@linear-code

linear-code Bot commented Oct 12, 2026

Copy link
Copy Markdown

PLAT-5848

Comment thread projects/fal/src/fal/toolkit/file/file.py Fixed
The isolate agent pickles raised exceptions back to the client, and
BaseException.__reduce__ re-invokes __init__(*args). With only the message
in args, unpickling failed on the missing failures argument.
@buraksenn
buraksenn marked this pull request as ready for review October 12, 2026 13:07

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and have nothing to add beyond the inline finding. Beyond that, I also checked the new fallback summary message (file.py:195-202) for the same kind of unbounded-size concern the PR addresses with its 4 KiB cap on CDN error bodies — but the summary only ever joins the small, fixed number of fallback attempts actually configured, so it isn't a practical issue.

Extended reasoning...

The PR adds FileUploadFallbackException (exceptions.py) to aggregate upload failures across fallback attempts in file.py's _try_with_fallback, and enriches HTTPError messages in providers/fal.py with up to 4 KiB of server response body across six upload-initiation paths; it's diagnostics-only and doesn't touch auth, crypto, or data-access logic. Beyond the confirmed inline finding, I checked the uncapped string concatenation of per-attempt failure messages at file.py:202 against the PR's own 4096-byte cap for CDN response bodies and ruled it out, since the fallback chain is short in practice.

Comment thread projects/fal/src/fal/toolkit/exceptions.py Outdated
Drop the unused `failures` attribute and custom constructor; the message
already lists every attempt. Stop dumping the traceback on each fallback:
one line per failed attempt is enough, and the final exception chains the
last cause.
…pload-fallback-errors

# Conflicts:
#	projects/fal/src/fal/toolkit/file/file.py
Comment thread projects/fal/src/fal/toolkit/file/file.py Dismissed
@buraksenn
buraksenn merged commit e285869 into main Oct 13, 2026
18 of 30 checks passed
@buraksenn
buraksenn deleted the buraksenn/plat-5848-upload-fallback-errors branch October 13, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants