Skip to content

fix(extras): stop following redirects in the MicroVM request step - #1036

Merged
ParidelPooya merged 3 commits into
aws:mainfrom
Thanatos9404:fix/extras-microvm-no-redirects
Oct 20, 2026
Merged

ParidelPooya merged 3 commits into
aws:mainfrom
Thanatos9404:fix/extras-microvm-no-redirects

Conversation

@Thanatos9404

Copy link
Copy Markdown
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

Fixes #1001

Description

sendJob called fetch with the default redirect: "follow". For a 301, 302 or 303, fetch resent the job as a GET without the body, and the GET's 2xx counted as accepted, so the job never started. A redirect to another host also received the auth token.

This change passes redirect: "manual" and fails any 3xx at once with MicrovmRequestRejectedError, naming the Location, which mirrors the Python SDK's aws/aws-durable-execution-sdk-python#776. A 307 or 308 for a missing trailing slash, which used to work, now fails with that message. The README notes the behavior.

Demo/Screenshots

N/A

Checklist

  • I have filled out every section of the PR template
  • I have thoroughly tested this change

Testing

Unit Tests

Yes. New tests cover 301, 302, 303, 307 and 308 with and without a Location, plus one real fetch against a local server; they fail on main, and the extras suite, typecheck and Biome pass.

Integration Tests

No, the unit tests cover it.

Examples

Not applicable.

sendJob called fetch with the default redirect mode, "follow". For a
301, 302, or 303, fetch resends the POST as a GET without the job, and a
2xx for that GET counted as accepted. So the job never started and the
operation waited until its heartbeat timeout. A redirect to another host
also received the auth token headers.

The request now uses redirect "manual", and any 3xx fails the delivery
at once with MicrovmRequestRejectedError, naming the Location in the
message. This matches the Python SDK, see
aws/aws-durable-execution-sdk-python#776. The README documents the
behavior.
@Thanatos9404
Thanatos9404 temporarily deployed to ai-pr-review-runtime October 20, 2026 06:40 — with GitHub Actions Inactive

@ParidelPooya ParidelPooya 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.

Thanks for the fix. It closes #1001: on main a 302 turned the job into a GET and a 307 sent the token to another host, and with this PR both fail at once. The new tests fail on main too. A few changes, inline below.

Also, please add a line under [Unreleased] in CHANGELOG.md. A 307 or 308 for a missing trailing slash used to deliver the job and now fails, so callers need to set the path with its trailing slash.

A custom fetch that does not pass init.redirect on still followed a
redirect, so a 302 resolved with the GET's 200 and a 307 to another
origin received the auth token. The request step now fails when
response.redirected is true, before the status checks, and names
response.url. The fetch docstring says a custom fetch must honor
init.redirect.

The Location in the message, set by the server and checkpointed with the
step failure, now drops its query and fragment and is cut to 200
characters. A 3xx without a Location, such as a 304, names only the
status. CHANGELOG notes that a 307 or 308 for a missing trailing slash
now fails.
@Thanatos9404

Copy link
Copy Markdown
Contributor Author

Thanks, all four are in 23a4bfd. A redirect that a custom fetch followed (response.redirected) now fails before the status checks and names response.url, and the fetch docstring says a custom fetch must honor init.redirect. The target in the message drops its query and fragment and is cut to 200 characters, a 3xx without a Location names only the status, and there is a CHANGELOG line under Unreleased about the trailing slash.

@ParidelPooya ParidelPooya 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.

Thanks, all four changes look good. I reran the redirect probes on 23a4bfd: every 3xx fails at once with one POST, a followed redirect from a custom fetch now fails instead of resolving, a 3xx without Location names only the status, and a 15,000-character Location gives a 356-character message. Removing the response.redirected check, the query strip, or the length cap each fails a test.

@ParidelPooya
ParidelPooya merged commit b629df6 into aws:main Oct 20, 2026
16 checks passed
ParidelPooya added a commit to aws/aws-durable-execution-sdk-python that referenced this pull request Oct 23, 2026
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.
ParidelPooya added a commit to aws/aws-durable-execution-sdk-python that referenced this pull request Oct 23, 2026
* 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.
zhongkechen pushed a commit to aws/aws-durable-execution-sdk-python that referenced this pull request Oct 23, 2026
* 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.
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.

MicroVM: stop following redirects in the request step

2 participants