Skip to content

Fix response retention after cancelling client method calls - #7065

Open
remote-controlled-man wants to merge 2 commits into
flet-dev:mainfrom
remote-controlled-man:codex/prflow-4e4ca3816726
Open

remote-controlled-man wants to merge 2 commits into
flet-dev:mainfrom
remote-controlled-man:codex/prflow-4e4ca3816726

Conversation

@remote-controlled-man

@remote-controlled-man remote-controlled-man commented Nov 8, 2026 •

Copy link
Copy Markdown

Description

Cancelled client method calls can retain response data for the lifetime of a session. For example, cancelling a task awaiting page.get_package_info() leaves its request registered, so a later client response is stored even though no caller can consume it. The same mechanism handles methods such as Screenshot.capture(), whose responses can be much larger.

There is a second cancellation window after the response has arrived but before the waiting coroutine resumes. Removing only the pending request does not release that already-stored response.

Unregister the request and discard any unconsumed response in a finally block covering message sending, waiting, and result consumption. Cancellation still propagates to the caller. Successful results, client errors, timeouts, and session-close errors keep their existing behavior.

Test code

From sdk/python:

uv run --group test pytest packages/flet/tests/test_session_invoke_method.py

The tests exercise the real session request/response path, including page.get_package_info(). They assert that response objects become collectible after cancellation both before and after response delivery, and after timeout or session closure. Both cancellation orderings failed before their corresponding fixes.

Validation on macOS, Python 3.11.16:

  • All seven Python SDK unit-test directories (the complete packages/*/tests set): 1980 passed, 15 skipped, 0 failed in 31.91 seconds, including all four new regression cases.
  • All six configured pre-commit hooks passed for the three changed files.

The run reports FastAPI root_path deprecations, the unset pytest-asyncio fixture-scope warning, and the shallow clone's git describe warning. Cross-platform device tests are left to CI.

Type of change

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

Checklist

  • I signed the CLA.
  • I have performed a self-review of my own code.
  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • New and existing tests pass locally with my changes.
  • I have made corresponding documentation changes, if applicable. (No API change.)
  • I have added changelog entries for user-facing changes, if applicable.
  • I have updated release guide pages and website/sidebars.yml for breaking changes, removals, and deprecations, if applicable. (Not applicable.)

Screenshots

Not applicable.

Additional details

AI-assisted contribution using Codex. CLA status has not yet been verified for this account. Cross-platform device tests have not been run locally.

Summary by Sourcery

Release client method call resources reliably when requests are cancelled or fail.

Bug Fixes:

  • Ensure cancelled, timed-out, and session-closed client method calls release pending requests and unconsumed responses instead of retaining response data for the session lifetime.
  • Preserve the expected timeout error message on Python 3.10 and later.

Tests:

  • Add regression coverage for response cleanup when cancellation occurs before or after response delivery, as well as after timeouts and session closure.

Copilot AI balanced review requested due to automatic review settings November 8, 2026 17:13
@CLAassistant

CLAassistant commented Nov 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

….1.3

On Python 3.10 asyncio.wait_for raises asyncio.TimeoutError, which is not
the builtin TimeoutError, so invoke_method() timeouts skipped the
descriptive message. The test now asserts the builtin type and message.
@ndonkoHenri

Copy link
Copy Markdown
Contributor

Thanks for your contribution @remote-controlled-man - can you please check the CLA requirement?

@remote-controlled-man

Copy link
Copy Markdown
Author

Thanks for the Python 3.10 timeout correction and changelog update. I checked the current head 5c078e1 with those changes included.

The current CI merge ref (c2af620, merging 5c078e1 into 1a19e48) passes the Python SDK unit suite: Python 3.10 reports 1973 passed, 22 skipped, and Python 3.11 reports 1974 passed, 21 skipped. Both logs show all four new test_session_invoke_method.py cases passing.

The failing Android py3.12 job stops during assembleDebug, before the Flutter app connects:

Execution failed for task ':serious_python_android:downloadDistArchive_armeabi-v7a'.
Unable to determine the current character, it is not a string, number, array, or object
The current character read is '\u0000' with an int value of 0

Pytest then reports a fixture setup error because the Flutter process exited. This points to Android dependency/build setup rather than an assertion failure in the session cleanup tests; the log does not establish where the invalid data originated.

Could someone with Actions write access rerun that Android py3.12 job once? This contributor account has read-only repository access, so I cannot rerun it. The failed job, remaining queued checks and outstanding CLA are still unresolved.

@remote-controlled-man

Copy link
Copy Markdown
Author

Could a maintainer rerun all nine remaining failed jobs on the unchanged head 5c078e1, using Re-run failed jobs on both runs below? This expands my earlier Android-only request.

Workflow Python Failed jobs
Flet Build Test 3.12 Build (apk-macos)
Flet Build Test 3.13 Build (apk-macos), Build (macos), Build (ipa), Build (web-macos), Build (ios-simulator)
Flet Build Test 3.14 Build (apk-macos), Build (ipa)
Flet Test (on-device) 3.12 android (py3.12)

Both inspected workflow runs are at attempt 2: eight build jobs plus one Android device job remain failed.

The CLA check, Python SDK unit-test jobs for Python 3.10–3.15, and the macOS integration workflow have passed on this head.

I rechecked the Android log: it fails in :serious_python_android:downloadDistArchive_armeabi-v7a with a parser error during assembleDebug, before the Flutter app connects. That establishes an Android build/setup failure; I have not established that the other eight jobs share the same cause.

No code changes are included in this follow-up.

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.

4 participants