-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conversation
LocalStack Community integration with Pro 2 files 2 suites 1h 14m 47s ⏱️ Results for commit d79b272. |
There was a problem hiding this 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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 🤔
There was a problem hiding this comment.
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?
Motivation
Changes
function_error
in batch error payloadexecutedVersion
from batch error payload