Skip to content

fix: preserve empty dict handler output - #535

Open
chenlichao wants to merge 3 commits into
runpod:mainfrom
chenlichao:fix/empty-dict-handler-output
Open

chenlichao wants to merge 3 commits into
runpod:mainfrom
chenlichao:fix/empty-dict-handler-output

Conversation

@chenlichao

Copy link
Copy Markdown

Summary

  • Stop dropping an empty dict returned by a serverless handler from the normalized job result.
  • Add regression coverage for local /runsync and /status simulation paths returning output: {}.

Why this helps

Test Plan

  • pytest tests/test_serverless/test_modules/test_fastapi.py -q --no-cov (8 passed)

Note: a targeted run without --no-cov also had all 8 tests pass, but failed the repository-wide coverage threshold because only one test file was selected.

@capy-ai

capy-ai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@promptless

promptless Bot commented Jul 9, 2026

Copy link
Copy Markdown

Promptless prepared a documentation update related to this change.

Triggered by runpod-python PR #535

Added a "Handler return values" section to the Serverless handler functions docs, explaining how a handler's return value maps to the job output field — dictionaries pass through directly, other types are wrapped, and an empty dictionary is a valid return that completes as COMPLETED with output: {}.

Review: Document Serverless handler return values

@chenlichao

Copy link
Copy Markdown
Author

bumping this for a first review — one-line fix preserving empty dict handler output, no behavioral regression intended.

@chenlichao

Copy link
Copy Markdown
Author

Weekly bump — still waiting on a first review for this one-line fix preserving empty-dict handler output. No behavioral regression, happy to adjust.

@chenlichao

Copy link
Copy Markdown
Author

Weekly bump — one-line fix preserving empty-dict handler output, with its test; still waiting on a first review.

@chenlichao

Copy link
Copy Markdown
Author

Friendly ping for a review — preserves empty-dict handler output instead of treating it as no result; tests included.

…key pops

Directly removing the empty-output pop breaks test_job_with_errors: popping
'error'/'refresh_worker' can empty the dict, and main expects those results
unchanged ({'error': 'test'} must not gain 'output': {}). Track whether the
handler itself returned {} and keep 'output' only in that case. Strengthen
the refresh_worker test to assert the exact result {'stopPod': true}.
@chenlichao

Copy link
Copy Markdown
Author

Updated the branch: merged current main (70f2de42) in — run_job moved under #570 (prestart hooks) and tests/.../test_job.py changed under #538 (volume cache), so the branch had gone stale/conflicting against main.

While resolving, I also tightened the fix itself. Directly deleting the empty-output pop changes results where popping error/refresh_worker empties the dict: a handler returning {"error": "test"} would become {"output": {}, "error": "test"} instead of the expected {"error": "test"}, and main's test_job_with_errors fails on exactly that (1 failed, 36 passed when I reverted to the delete-only version). The fix now tracks whether the handler itself returned {} and keeps output only in that case, so:

  • handler returns {} → {"output": {}} preserved (the point of this PR, covered by the new test_fastapi.py cases)
  • handler returns {"error": ...} / {"refresh_worker": true} → results unchanged ({"error": "test"}, {"stopPod": true}; the latter assertion was strengthened to check the exact dict)

Net diff vs main: rp_job.py +3/-1, plus the test_fastapi.py additions and one strengthened assertion in test_job.py.

Tests: full suite via make test — 736 passed, coverage gate green.

This branch has not been deployed

No deployments
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.

Returning empty object from handler raises KeyError exception within framework

1 participant