Skip to content

Preserve bound args for upload handlers#6602

Open
puneetdixit200 wants to merge 8 commits into
reflex-dev:mainfrom
puneetdixit200:fix/upload-bound-event-args
Open

Preserve bound args for upload handlers#6602
puneetdixit200 wants to merge 8 commits into
reflex-dev:mainfrom
puneetdixit200:fix/upload-bound-event-args

Conversation

@puneetdixit200
Copy link
Copy Markdown

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

Fixes #5290.

EventHandler.__call__ returned immediately when it saw rx.upload_files(...) or rx.upload_files_chunk(...), so any other bound arguments supplied to the same event handler were dropped. This keeps the upload client-handler spec, but appends normal bound handler arguments to the resulting EventSpec so on_drop=State.handle_upload(rx.upload_files(...), field) preserves field.

AI-assisted: OpenAI Codex, with local review and verification.

Testing

  • Red before fix: uv run pytest tests/units/components/core/test_upload.py::test_upload_files_preserves_bound_event_args -q failed because field was missing from the upload event args.
  • uv run pytest tests/units/components/core/test_upload.py -q
  • uv run pytest tests/units/test_event.py -q
  • uv run pytest tests/units/test_app.py -q -k 'upload and not compile_writes_upload_files_provider_app_wrap'
  • uv run ruff check packages/reflex-base/src/reflex_base/event/__init__.py tests/units/components/core/test_upload.py
  • uv run pyright packages/reflex-base/src/reflex_base/event/__init__.py tests/units/components/core/test_upload.py (0 errors, 1 existing warning in event/__init__.py)
  • git diff --check

Note: uv run pytest tests/units/test_app.py -q -k upload currently fails in unrelated test_compile_writes_upload_files_provider_app_wrap with AttributeError: 'DynamicState' object has no attribute 'dynamic'; the remaining 18 upload-selected app tests pass with that case excluded.

@puneetdixit200 puneetdixit200 requested a review from a team as a code owner June 4, 2026 03:40
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR fixes issue #5290 by threading extra bound handler arguments all the way through the upload pipeline — from the Python compile-time EventSpec, through the JavaScript uploadFiles client, and into the backend event dispatch for both buffered and streaming upload paths.

  • Python (event/__init__.py): EventHandler.__call__ now collects non-upload bound args into payload, detects reserved-name clashes and duplicate FileUpload args at build time, and appends a __reflex_event_arg_names manifest key so the JS client knows exactly which payload keys to forward.
  • JavaScript (state.js, upload.js): applyRestEvent reads the manifest and assembles extra_args; uploadFiles prepends them as a JSON form field before the file parts.
  • Backend (_upload.py): _decode_event_args validates the form field as a JSON object (400 on malformed/non-object); both the buffered and streaming paths parse and forward the args into the dispatched Event.payload.

Confidence Score: 5/5

Safe to merge; the fix is complete end-to-end with both unit and integration test coverage.

The Python compile-time path, the JS client, and both backend upload paths are all updated consistently. The previously flagged gaps are fully addressed. No regressions in the normal no-extra-args path.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/event/init.py Core fix: EventHandler.call now collects extra bound args alongside the upload EventSpec, adds a __reflex_event_arg_names manifest key, and detects reserved-name clashes and duplicate upload args at build time.
packages/reflex-components-core/src/reflex_components_core/core/_upload.py Backend updated for both buffered and streaming paths: _decode_event_args validates JSON dict, _buffered_upload_args reads the form field for non-chunked uploads, and _UploadChunkMultipartParser gains on_args_ready callback + size-bounded buffering for the streaming path.
packages/reflex-base/src/reflex_base/.templates/web/utils/state.js applyRestEvent now reads __reflex_event_arg_names from the event payload and collects only those keys into extra_args before passing them to uploadFiles.
packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js uploadFiles accepts new extra_args parameter and, when non-empty, prepends a __reflex_event_args JSON form field to the multipart body before the file parts.
tests/units/components/core/test_upload.py Comprehensive new unit tests covering compile-time EventSpec preservation, reserved-name clash, _decode_event_args validation, and streaming parser edge cases.
tests/integration/test_upload.py Two new integration test paths verifying end-to-end delivery of bound args for both buffered and streaming upload handlers.

Reviews (3): Last reviewed commit: "fix: carry upload bound args as a form f..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/event/__init__.py Outdated
@puneetdixit200 puneetdixit200 force-pushed the fix/upload-bound-event-args branch from 22f9b26 to 9da7528 Compare June 4, 2026 05:17
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 4, 2026

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing puneetdixit200:fix/upload-bound-event-args (4ce6406) with main (31f785e)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

FarhanAliRaza and others added 4 commits June 4, 2026 23:58
Args bound to an upload handler (e.g. State.on_drop(rx.upload_files(...),
field)) were preserved in the compiled event spec but never reached the
backend handler, since uploads travel over a REST endpoint instead of the
socket. The client now forwards the named extra args via a URL-encoded
JSON header, which the upload endpoint decodes and merges into the event
payload. Bound names that clash with reserved upload keys are rejected at
build time. Fixes reflex-dev#5290.
@puneetdixit200
Copy link
Copy Markdown
Author

The runtime upload path is covered on the current head now: the compiled event carries __reflex_event_arg_names, applyRestEvent forwards those keys to uploadFiles, uploadFiles sends them as Reflex-Event-Args, and the backend upload handlers merge that header into both buffered and chunk upload events.

Focused verification: uv run pytest tests/units/components/core/test_upload.py -k "upload_files_preserves_bound_event_args or upload_files_multiple_upload_args_raises or upload_files_bound_arg_reserved_name_raises" -q -> 3 passed, 12 deselected.

@FarhanAliRaza
Copy link
Copy Markdown
Contributor

@greptile please rereview.

Comment thread packages/reflex-components-core/src/reflex_components_core/core/_upload.py Outdated
Validate the reflex-event-args header before parsing form data so a
malformed or non-object JSON payload yields a 400 instead of a 500.
… them

Bound handler args were sent in a URL-encoded HTTP header, which the
streaming chunk parser never read, so chunked uploads dropped them and the
header path was capped by server limits. Move the args into a leading
__reflex_event_args multipart field parsed ahead of the first file chunk,
dispatching the handler once it is read. The field is size-bounded and
rejected if it arrives after a file part.
@FarhanAliRaza
Copy link
Copy Markdown
Contributor

@greptile please rereview.

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.

Cannot pass additional arguments to rx.upload on_drop handler

2 participants