Skip to content

fix(testing): match service for error size check and history redaction - #824

Merged
nvasiu merged 1 commit into
aws:mainfrom
nvasiu:fix/testing-error-size-and-history-redaction
Oct 26, 2026
Merged

fix(testing): match service for error size check and history redaction#824
nvasiu merged 1 commit into
aws:mainfrom
nvasiu:fix/testing-error-size-and-history-redaction

Conversation

@nvasiu

@nvasiu nvasiu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

N/A

Description of changes:

Fix two places where the Python local runner behaves differently than the service.

validators/checkpoint.py

  • Before, for an update that carried an error, the runner measures that the error object itself is <= than 32KB.
  • After, for an update that carries an error, the runner serializes the whole update and checks that it is <= than 1MB. Which matches the service validation.
    • The error message says "Error object size must be less than or equal to 1048576 bytes." even though it is measuring the entire update. This matches the message that the service gives.

model.py

  • Before, when history was requested without execution data (include_execution_data is false) events would still include the error payload while reporting Truncated: true.
  • After, every error event includes the error payload when execution data is requested (include_execution_data is true).
    • ContextFailed, CallbackFailed and CallbackTimedOut now pass the include flag. Before, they never passed it so they would always report Truncated: true even when data was requested.

And updated tests to match new behaviour.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@nvasiu
nvasiu temporarily deployed to ai-pr-review-runtime October 23, 2026 23:14 — with GitHub Actions Inactive
@nvasiu
nvasiu force-pushed the fix/testing-error-size-and-history-redaction branch 2 times, most recently from 9856a34 to a1cc27d Compare October 23, 2026 23:48
yaythomas
yaythomas requested changes Oct 24, 2026
View reviewed changes

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

The new boundary tests make the size rule clear. The history changes omit error payloads when callers exclude execution data. I found issues in Unicode handling, successful-result handling, type checking, and the wait-history test. I think these need changes before approval. The commit subject also exceeds the repository’s 50-character limit.

Comment thread ...hon-testing/src/aws_durable_execution_sdk_python_testing/checkpoint/validators/checkpoint.py
Comment thread packages/aws-durable-execution-sdk-python-testing/tests/event_factory_test.py
Comment thread packages/aws-durable-execution-sdk-python-testing/tests/event_factory_test.py Outdated
@yaythomas

yaythomas commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

The commit subject contains 70 characters. The repository allows at most 50. The commit-validation job therefore fails. Can you shorten the subject to 50 characters or fewer?

@yaythomas yaythomas mentioned this pull request Oct 25, 2026
Merged
3 tasks done
@nvasiu
nvasiu force-pushed the fix/testing-error-size-and-history-redaction branch from a1cc27d to 36d3a12 Compare October 26, 2026 18:57
@nvasiu

nvasiu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@yaythomas
Updated to address comments, details in replies to each comment.

@nvasiu
nvasiu force-pushed the fix/testing-error-size-and-history-redaction branch from 36d3a12 to a7037e5 Compare October 26, 2026 20:23
yaythomas
yaythomas reviewed Oct 26, 2026
View reviewed changes
Comment thread ...s-durable-execution-sdk-python-testing/src/aws_durable_execution_sdk_python_testing/model.py
include: bool = False, # noqa: FBT001, FBT002
) -> EventInput:
input: str | None = start_durable_execution_input.input
input: str | None = start_durable_execution_input.input if include else None

@yaythomas yaythomas Oct 26, 2026

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.

The latest revision changes EventInput.from_start_durable_execution_input(), but no repository code or test calls this method. So all tests still pass if someone removes the new if include else None branch. CONTRIBUTING rule 4 requires each new branch to have a test that fails without it.

@nvasiu nvasiu Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a test for this.

@nvasiu
fix(testing): match service error limits
853eebf 
Measure the whole update in UTF-8 bytes against 1,048,576 when it
carries an error, instead of only the error object against 32,768.

Include the error payload in history events only when execution data
is requested.
@nvasiu
nvasiu force-pushed the fix/testing-error-size-and-history-redaction branch from a7037e5 to 853eebf Compare October 26, 2026 22:25
yaythomas
yaythomas approved these changes Oct 26, 2026
View reviewed changes

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

:shipit:

@nvasiu
nvasiu merged commit 2e5135f into aws:main Oct 26, 2026
24 of 28 checks passed
@nvasiu
nvasiu deleted the fix/testing-error-size-and-history-redaction branch October 26, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaythomas yaythomas yaythomas approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nvasiu @yaythomas