Repository navigation
fix(extras): record the session resume mode - #811
Conversation
The resume mode says whether each session job checks the MicroVM state, and resumes a suspended MicroVM, before delivery. The session computed it from the current config on every replay. 1. The worker reads its idle time once, from the run hook payload of the launch. RunMicrovm sets the idle policy once, at the launch. 2. So a deploy that turns auto_suspend_on_idle off, or removes idle_policy, does not stop a running MicroVM from suspending. 3. The next job then skipped the state check. The suspended endpoint answered 502. 4. The request step retries a 502 as a MicroVM that is still starting. So the job failed with MicrovmDeliveryError only after both retry tiers. A session launch now writes the resume mode to the launch checkpoint, as "resumeBeforeDelivery": true or false. Replay follows the recorded mode, not the current config. The JavaScript SDK writes the same field, in aws/aws-durable-execution-sdk-js#1042. LaunchResult reads the field only when it is a bool. In Python True == 1, so a check by equality would accept 1. A checkpoint without the field, or with another value, falls back to the current config. A microvm() launch records no resume mode. So its checkpoint keeps only microvmId, endpoint, and delivery. Fixes #798
With auto_suspend_on_idle=True and no auto_suspend_idle_time, the default idle time is the session timeout if that is shorter than 60 seconds. The idle time must be at least 10 seconds. So a session whose timeout is under 10 seconds does not suspend by default, and it records "resumeBeforeDelivery": false. The README did not state this case. The JavaScript README does.
| result = run(handler, None, [("build", succeed_with(1)), ("test", succeed_with(2))]) | ||
|
|
||
| assert outcome(result)["result"]["test"] == 2 | ||
| assert len(invocations) > 1 |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ajwnsebjasbacyyspt6wbmvvj4
[P2] Guarantee a replay before delivering the second job
The run fixture completes callbacks as soon as wait_for_callback sees their creation; it does not wait for suspension. If the first callback completes before the request step checkpoints success, that checkpoint supplies the callback result and both jobs can be delivered in the first invocation. A suspension while awaiting the second callback then satisfies len(invocations) > 1 without exercising delivery under the changed config. If both callbacks complete early, the assertion instead fails despite correct behavior. Both new deployment tests share this race. Pause the execution before completing the first callback, then complete it and resume, and assert that the second request executes in a later invocation.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewOne P2 test-reliability finding at Reviewed commit |
* test(extras): end the invocation between two jobs Two session tests of #811 check the resume mode across a deploy. The handler counts its invocations. The first invocation uses the launch's config. Every later invocation uses the deployed config. Each test runs two jobs and asserts len(invocations) > 1. 1. The run fixture completes each callback as soon as the job creates it. 2. A checkpoint response carries every operation that changed since the last response. 3. So the first callback's result can reach the first invocation. 4. So both jobs can be delivered in the first invocation, with the launch's config. 5. A suspension while the second job waits then satisfies len(invocations) > 1. No job is delivered after the deploy. 6. If both callbacks complete early, the execution ends in one invocation. The assertion then fails. The Codex review of #811 reported this race. In 65 local runs of the old test body, the second job was delivered in invocation 2. The test body did not ensure that order. The two tests now run a durable wait between the jobs. A durable wait always ends the invocation. The local runner skips the wait's time. So the second job is delivered in a later invocation, after the deploy, in every run. Each test also records the invocation that sent each request. It asserts that the first request was sent in invocation 1, and the second request in a later invocation. The tests fail against the session code before #811. The failures are at the GetMicrovm count: 1 instead of 3, and 1 instead of 0. * docs(extras): state the JS redirect behavior The README said that the JavaScript operation follows redirects. aws/aws-durable-execution-sdk-js#1036 changed that. The JavaScript request step now passes redirect: "manual". It fails every 3xx at once. The operation then raises MicrovmDeliveryError. So the README now states that the JavaScript operation also fails every 3xx at once, with MicrovmDeliveryError. * docs(extras): fix the send_job redirect docstring The send_job docstring said that the JavaScript operation follows redirects. aws/aws-durable-execution-sdk-js#1036 changed that. The JavaScript request step now fails every 3xx at once. So the docstring contradicted the corrected README sentence. The docstring now states that neither operation follows a redirect. It then states why: a followed 301, 302, or 303 would send a GET without the job, and a followed 307 or 308 to another host would receive the auth token. * test(extras): name the wait between the two jobs The handler two_jobs_in_two_invocations runs two jobs with a durable wait between them. The execution runs 4 invocations, not 2. So the name is now two_jobs_with_a_wait_between. The SDK's wait returns without a suspension if the START checkpoint response marks the wait SUCCEEDED. The local runner always answers a START with STARTED. So the docstring now states that a durable wait ends the invocation in the local runner. A request can also time out without an answer. So the comment in _InvocationLog.append now says that the POST gets an answer or times out, and that the fake endpoint answers at once. The sent_in assertions require at least 2 invocations. So the two deploy tests no longer assert len(invocations) > 1.
* test(extras): end the invocation between two jobs Two session tests of #811 check the resume mode across a deploy. The handler counts its invocations. The first invocation uses the launch's config. Every later invocation uses the deployed config. Each test runs two jobs and asserts len(invocations) > 1. 1. The run fixture completes each callback as soon as the job creates it. 2. A checkpoint response carries every operation that changed since the last response. 3. So the first callback's result can reach the first invocation. 4. So both jobs can be delivered in the first invocation, with the launch's config. 5. A suspension while the second job waits then satisfies len(invocations) > 1. No job is delivered after the deploy. 6. If both callbacks complete early, the execution ends in one invocation. The assertion then fails. The Codex review of #811 reported this race. In 65 local runs of the old test body, the second job was delivered in invocation 2. The test body did not ensure that order. The two tests now run a durable wait between the jobs. A durable wait always ends the invocation. The local runner skips the wait's time. So the second job is delivered in a later invocation, after the deploy, in every run. Each test also records the invocation that sent each request. It asserts that the first request was sent in invocation 1, and the second request in a later invocation. The tests fail against the session code before #811. The failures are at the GetMicrovm count: 1 instead of 3, and 1 instead of 0. * docs(extras): state the JS redirect behavior The README said that the JavaScript operation follows redirects. aws/aws-durable-execution-sdk-js#1036 changed that. The JavaScript request step now passes redirect: "manual". It fails every 3xx at once. The operation then raises MicrovmDeliveryError. So the README now states that the JavaScript operation also fails every 3xx at once, with MicrovmDeliveryError. * docs(extras): fix the send_job redirect docstring The send_job docstring said that the JavaScript operation follows redirects. aws/aws-durable-execution-sdk-js#1036 changed that. The JavaScript request step now fails every 3xx at once. So the docstring contradicted the corrected README sentence. The docstring now states that neither operation follows a redirect. It then states why: a followed 301, 302, or 303 would send a GET without the job, and a followed 307 or 308 to another host would receive the auth token. * test(extras): name the wait between the two jobs The handler two_jobs_in_two_invocations runs two jobs with a durable wait between them. The execution runs 4 invocations, not 2. So the name is now two_jobs_with_a_wait_between. The SDK's wait returns without a suspension if the START checkpoint response marks the wait SUCCEEDED. The local runner always answers a START with STARTED. So the docstring now states that a durable wait ends the invocation in the local runner. A request can also time out without an answer. So the comment in _InvocationLog.append now says that the POST gets an answer or times out, and that the fake endpoint answers at once. The sent_in assertions require at least 2 invocations. So the two deploy tests no longer assert len(invocations) > 1.
* refactor(extras): drop the resume mode fallback Only builds from git main before #811 wrote a session launch checkpoint without resumeBeforeDelivery. The package was never published. So a missing field now reads as False, and replay ignores the current config. * test(extras): cover a non-bool resume mode The session test now covers a missing, "true", and 1 resume mode. The comment and docstring say only True is a resume mode, as the review asked. * fix(extras): keep only a bool resume mode LaunchResult.from_dict reads any non-bool resume mode as None again. This addresses the review, and the restored test covers the case.
Fixes #798.
Summary
A
microvm_sessionnow writes its resume mode to the launch checkpoint as"resumeBeforeDelivery": trueorfalse. Replay follows the recorded mode, not the current config. This matches aws/aws-durable-execution-sdk-js#1042.Why
The resume mode says whether each session job checks the MicroVM state, and resumes a suspended MicroVM, before delivery. The session computed it from the current config on every replay.
auto_suspend_on_idleoff, or removesidle_policy, does not stop a running MicroVM from suspending.MicrovmDeliveryErroronly after both retry tiers.Changes
LaunchResulthas a new field,resume_before_delivery: bool | None. Its wire name isresumeBeforeDelivery.from_dictreads the field only whenisinstance(value, bool). In PythonTrue == 1, so a check by equality would accept1.to_dictomits the key when the value isNone. So themicrovm()checkpoint is unchanged._SessionScope.resume_before_deliveryis renamed toresume_from_config, because it now reflects only the current config.lambda:GetMicrovmandlambda:ResumeMicrovmuntil every session that launched with suspending on has ended. A session whosetimeoutis under 10 seconds does not suspend by default.Checkpoint format
The JS and Python SDKs share the launch checkpoint format. So both SDKs must use the exact field name.
Session launch:
{"microvmId":"mvm-1","endpoint":"mvm-1.lambda-microvm.us-east-1.on.aws","delivery":"http","resumeBeforeDelivery":true}microvm()launch, unchanged:{"microvmId":"mvm-1","endpoint":"mvm-1.lambda-microvm.us-east-1.on.aws","delivery":"http"}Python writes the same keys in the same order. Its JSON has spaces after
:and,, as it did before for the other keys. A JS reader parses it, and a Python reader parses the JS form.The extras package is not published to PyPI. So no released version wrote a launch checkpoint without the field. The fallback covers checkpoints from a build of git main.
Testing
hatch run dev-extras:cov: 279 passed, 100 % coverage. On origin/main: 259 passed, 100 % coverage.dev-extras:typecheck,hatch fmt --check, and the commit lint pass.hatch run test:cov: 5408 passed, 2 skipped.MicrovmDeliveryErrorafter 4 attempts. On this branch it resumes the MicroVM and succeeds.isinstance, always write the key, fall back toTrueorFalse, record nothing, coerce withbool().Known gap, not changed here
A launch step that fails, and is retried after a deploy, computes the resume mode from the new config. If the first attempt created the MicroVM and only its response was lost, the recorded mode can be wrong. The JavaScript operation has the same gap. A fix belongs in both SDKs together.