Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESM-v2: Remove reference to unbound variable #11495

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

gregfurman
Copy link
Contributor

Motivation

Changes

  • Remove incorrect + unbound reference to function_error in batch error payload
  • Remove executedVersion from batch error payload

@gregfurman gregfurman added type: bug Bug report aws:lambda AWS Lambda labels Sep 10, 2024
@gregfurman gregfurman added this to the 3.8 milestone Sep 10, 2024
@gregfurman gregfurman added the semver: patch Non-breaking changes which can be included in patch releases label Sep 10, 2024
Copy link

LocalStack Community integration with Pro

    2 files      2 suites   1h 14m 47s ⏱️
2 502 tests 2 200 ✅ 302 💤 0 ❌
2 504 runs  2 200 ✅ 304 💤 0 ❌

Results for commit d79b272.

@gregfurman gregfurman marked this pull request as ready for review September 10, 2024 16:34
@gregfurman gregfurman self-assigned this Sep 10, 2024
Copy link
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @gregfurman 👍

@@ -85,8 +84,6 @@ def send_events(self, events: list[dict]) -> dict:
"requestId": invoke_result["ResponseMetadata"]["RequestId"],
"exceptionType": "BadRequest",
"resourceArn": self.target_arn,
"functionError": function_error,
Copy link
Member

Choose a reason for hiding this comment

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

I see that this was the issue. Reverting this change fixes it because the executedVersion and function_error is not needed for batch item failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly. Not sure how I missed this when running the tests initially 🤔

Copy link
Member

Choose a reason for hiding this comment

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

What can we do to catch such regressions in the future? Would better test coverage for batch failure handling catch this?

@gregfurman gregfurman merged commit 9663d6d into master Sep 10, 2024
38 of 41 checks passed
@gregfurman gregfurman deleted the fix/esm/add-function-error branch September 10, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:lambda AWS Lambda semver: patch Non-breaking changes which can be included in patch releases type: bug Bug report
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants