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

fix: Avoid failure if a handler argument is provided to the lambda #99

Merged
merged 4 commits into from Jul 1, 2022

Conversation

timleslie
Copy link
Contributor

@timleslie timleslie commented Jul 1, 2022

What

This PR adds a workaround for some unexpected behaviour in the combination of LocalStack and clap which can cause environment variable parsing to fail when running Image based lambdas in LocalStack.

Why

When running Image based lambdas in LocalStack, I observed that a command line argument of handler.handler was being provided to the rust binary process. I need to further investigate how/why this is being passed in. I believe it to be a bug, but I haven't confirmed that.

This command line value was in turn parsed by clap as part of our environment parsing. The behaviour of clap was to assign this command line value to the first element of the Env struct, and then ignore any environment variable provided for that element. This caused the wrong value to be provided to the Env struct (although it was considered valid, as my use case expected a String value for this first element). The code would then attempt to parse the HandlerEnv, at which point it would fail, as HandlerEnv.record_concurrency is a usize value, which cannot be parsed from a string.

This PR works around this situation by always passing an empty iterator to Env::try_parse_from(), since we never care about any command line arguments anyway.

@timleslie timleslie marked this pull request as ready for review July 1, 2022 07:13
@timleslie timleslie requested a review from a team as a code owner July 1, 2022 07:13
Copy link
Contributor

@rfk rfk left a comment

Choose a reason for hiding this comment

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

Makes sense to me, nice work tracking down what was happening here!

@timleslie timleslie merged commit e8b1641 into main Jul 1, 2022
@timleslie timleslie deleted the fix/ignore-handler-arg branch July 1, 2022 07:28
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.

None yet

2 participants