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

Handle network errors in Lambda client #100

Closed
wants to merge 2 commits into from

Conversation

ziltodian
Copy link

The Lambda client treats all errors (apart from Lambda function errors) as AWS errors. This small change checks the response.error_code to see if the error is an HTTP error or some other kind of network error (https://k6.io/docs/javascript-api/error-codes/). It also exports both AWSError and NetworkError. Our use case for this is that sometimes we encounter network errors where the Lambda invocation request never makes it to AWS. We need to identify when this occurs so we can exclude these failures from our test results. Currently we check if the error.name is "AWSError" and error.code is empty string. We'd like to be able to check for an instance of NetworkError instead.

@CLAassistant
Copy link

CLAassistant commented Apr 29, 2024

CLA assistant check
All committers have signed the CLA.

@ziltodian ziltodian marked this pull request as ready for review April 29, 2024 23:32
@ziltodian ziltodian requested a review from a team as a code owner April 29, 2024 23:32
@ziltodian ziltodian requested review from mstoykov and codebien and removed request for a team April 29, 2024 23:32
@oleiade
Copy link
Member

oleiade commented May 24, 2024

Hi @ziltodian 👋🏻

Thanks a lot for your contribution 🙇🏻

We looked thoroughly into your use-case and PR with @codebien and reached the conclusion we'd like to pursue going in this direction indeed. However, we'd like to go even step further, and would like to propose more changes to your PR if you have the capacity to contribute further to it.

At the moment, the NetworkError you have defined is a bit of a catch-all error. And while in your specific case, it would likely good enough, we'd like to provide more granularity to users using the library as we're at it, and eventually (not in this PR) extend to even more of the clients we provide.

Dedicated network error types

As you might have found in the k6 documentation, the error_code that k6 returns offers some level of granularity and context as to what happened.

The error codes described in the doc fall essentially into X categories:

  • DNS errors
  • TCP errors
  • TLS errors
  • HTTP2 specific errors
  • and as you found out, HTTP4xx and HTTP5xx errors

As a result, we'd like to define DNSError, TCPError, TLSError, and HTTP2Error types, which would be returned in case the error code received by k6 querying AWS is within the range defined in the range.

Custom lambda error types

But what should we do with the standard HTTP error status codes, then? We suggest creating dedicated LambdaError type, following the same convention as established by S3Client, with a dedicated mapping between the error name, and the HTTP status code as defined in the AWS API documentation.

Next steps

We believe this would be the right design moving forward, not only for the lambda client, but for the whole library. In this PR our expectation is that we would modify only the lambda client (and maybe the internal/error.ts file too). But that if we're happy with the result here, we would expand it to the whole codebase at a later point in time.

Is this something you'd have the capacity to work on? We wouldn't be able to merge the PR in its current state, but if you don't have the capacity to work on the proposed changes, let us know, and we'll try to contribute some of the changes we had in mind ourselves.

Let us know 🙇🏻

@mstoykov mstoykov requested review from oleiade and removed request for mstoykov May 29, 2024 13:54
# Conflicts:
#	dist/aws.js
#	dist/aws.js.map
#	dist/event-bridge.js
#	dist/event-bridge.js.map
#	dist/index.js
#	dist/index.js.map
#	dist/kinesis.js
#	dist/kinesis.js.map
#	dist/kms.js
#	dist/kms.js.map
#	dist/lambda.js
#	dist/lambda.js.map
#	dist/s3.js
#	dist/s3.js.map
#	dist/secrets-manager.js
#	dist/secrets-manager.js.map
#	dist/signature.js
#	dist/signature.js.map
#	dist/sqs.js
#	dist/sqs.js.map
#	dist/ssm.js
#	dist/ssm.js.map
@oleiade
Copy link
Member

oleiade commented Jun 17, 2024

Quick heads-up that we have startd preparing a branch introducing the base error types I mentioned earlier, and will open a PR soon-ish.

@oleiade
Copy link
Member

oleiade commented Jul 30, 2024

Hi @ziltodian 👋🏻

Thanks a lot, again, for your contribution 🙇🏻 We have shipped custom error types that match the k6 error codes as described above. Whenever performing lambda operations, you should be able to try/catch and match the error type you receive, if you wish to distinguish network errors from aws errors.

I invite you to check error.ts and its definitions to get a feeling for the new error kinds, and also notice AWSClient now have a base handleError method which does the general handling.

I will close this PR, but feel free to open an issue if you have further questions or needs 🙇🏻

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.

3 participants