Skip to content

Add error handling if lambda logs are not received from the environment #12521

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2025
Merged

Add error handling if lambda logs are not received from the environment #12521

merged 1 commit into from
Apr 17, 2025

Conversation

dfangl
Copy link
Member

@dfangl dfangl commented Apr 14, 2025

Motivation

Currently, if the lambda environment, for currently unknown reasons, does not post logs to LS for an invocation, we error the invocation while trying to print logs.

We should avoid failing the entire invocation, but clearly log a warning, and ideally log the container execution logs as well, to be able to determine the root cause of the missing logs.

Changes

  • Prevent invocation from failing when we cannot print logs to the console
  • Add additional logging to allow investigation of the root cause in the future

@dfangl dfangl added the semver: patch Non-breaking changes which can be included in patch releases label Apr 14, 2025
@dfangl dfangl self-assigned this Apr 14, 2025
@dfangl dfangl changed the title Add error handling if lambda logs are not received from the environment" Add error hanlding if lambda logs are not received from the environment Apr 14, 2025
@dfangl dfangl changed the title Add error hanlding if lambda logs are not received from the environment Add error handling if lambda logs are not received from the environment Apr 14, 2025
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 30m 39s ⏱️ - 22m 27s
3 169 tests  - 1 183  2 925 ✅  - 1 081  244 💤  - 102  0 ❌ ±0 
3 171 runs   - 1 183  2 925 ✅  - 1 081  246 💤  - 102  0 ❌ ±0 

Results for commit 55ff2406. ± Comparison against base commit 1724451.

This pull request removes 1183 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

joe4dev
joe4dev approved these changes Apr 14, 2025
View reviewed changes
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.

More resilient against missing logs 🩹 🚀

localstack-core/localstack/services/lambda_/invocation/version_manager.py
invocation_result.request_id,
self.function_arn,
execution_env.get_prefixed_logs(),
Copy link
Member

Choose a reason for hiding this comment

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

Good idea to present actionable output 👍

Hopefully, not too long 🤞

@dfangl dfangl merged commit 3ea87fe into master Apr 17, 2025
38 checks passed
@dfangl dfangl deleted the lambda/no-log-error-handling branch April 17, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe4dev joe4dev joe4dev approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman is a code owner

Assignees

@dfangl dfangl

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dfangl @joe4dev