Skip to content

Consider optional headers for APIGatewayRequest #73

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

Closed
twistinside opened this issue Nov 11, 2024 · 4 comments · Fixed by #80
Closed

Consider optional headers for APIGatewayRequest #73

twistinside opened this issue Nov 11, 2024 · 4 comments · Fixed by #80
Assignees
Labels
kind/enhancement Improvements to existing feature.

Comments

@twistinside
Copy link

Expected behavior

Making headers optional on APIGatewayRequest allows for trivial testing via test console in AWS. While it's unlikely a request will not have headers in real world usage, it's trivial to reproduce in API Gateway console during testing.

I ran into this use case while testing the integration between a simple GET endpoint and my Lambda. My lambda doesn't interact with headers at all, so while it's trivial to add one for testing, I'm curious if there's a specific reason that the headers aren't optional, and if they can be updated.

Actual behavior

2024-11-11T19:55:58+0000 warning Lambda : lifecycleIteration=0 [AWSLambdaRuntimeCore] lambda handler returned an error: requestDecoding(Swift.DecodingError.valueNotFound(Swift.Dictionary<Swift.String, Any>, Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "headers", intValue: nil)], debugDescription: "Cannot get value of type Dictionary<String, Any> -- found null value instead", underlyingError: nil)))

Steps to reproduce

  1. Create any Lambda using the Swift Lambda Runtime that's triggered by API Gateway
  2. Make an empty request in the API Gateway console

If possible, minimal yet complete reproducer code (or URL to code)

The demo implementation at the SPM page will be adequate as long as it's triggered by API Gateway.

SwiftAWSLambdaRuntime version/commit hash

1.0.0-alpha

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2)
Target: arm64-apple-macosx15.0
Darwin MacBook-Pro.localdomain 24.1.0 Darwin Kernel Version 24.1.0: Thu Oct 10 21:03:15 PDT 2024; root:xnu-11215.41.3~2/RELEASE_ARM64_T6000 arm64
@sebsto
Copy link
Contributor

sebsto commented Nov 12, 2024

Hello, thank you for having reported this.

It seems a legit request to me.
But why making just HTTPHeaders optional? If you pass an empty JSON, all fields have to be marked optional. And that would apply for all event types.

Can you share an example of the input payload you give to your Lambda function during your tests ?

@sebsto sebsto added the kind/enhancement Improvements to existing feature. label Nov 12, 2024
@sebsto sebsto self-assigned this Nov 12, 2024
@twistinside
Copy link
Author

I am testing via the API Gateway console, which generates the HTTP request based on query strings and headers that are user provided. Here's the request as passed into my function once I add the headers that allow the runtime to deserialize:

{
  "resource": "/v1",
  "path": "/v1",
  "httpMethod": "GET",
  "queryStringParameters": null,
  "multiValueQueryStringParameters": null,
  "headers": {
    "foo": "bar"
  },
  "multiValueHeaders": {
    "foo": ["bar"]
  },
  "pathParameters": null,
  "stageVariables": null,
  "requestContext": {
    "resourceId": "3uky0x",
    "apiId": "wia2isnqag",
    "domainName": "testPrefix.testDomainName",
    "resourcePath": "/v1",
    "httpMethod": "GET",
    "requestId": "3fa0a0d6-ae9d-498a-9d12-f7acb8e3df89",
    "accountId": "260754460809",
    "stage": "test-invoke-stage",
    "identity": {
      "cognitoIdentityPoolId": null,
      "apiKey": "test-invoke-api-key",
      "userArn": "arn:aws:iam::260754460809:root",
      "cognitoAuthenticationType": null,
      "caller": "260754460809",
      "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.1 Safari/605.1.15",
      "user": "260754460809",
      "cognitoAuthenticationProvider": null,
      "sourceIp": "test-invoke-source-ip",
      "accountId": "260754460809"
    },
    "authorizer": null,
    "extendedRequestId": "BKYW1GjCSK4FrdA=",
    "path": "/v1"
  },
  "body": null,
  "isBase64Encoded": false
}

Based on this limited use case, I will at least make the headers and multiValueHeaders properties optional. I can make any other similar updates you suggest while I'm at it.

Here is an example of the failure I'm referring to in the console:
API Gateway - Failure

And here is the success after adding arbitrary headers
API Gateway - Success

@twistinside
Copy link
Author

I've thought about this more, and the question I think comes down to expected behavior for deserialization in Swift. There are many ways to pass data to the lambda, and we can't really ensure that they'll conform to APIGatewayReqeust. In reality, in most cases that means the lambda code will not be successfully runnable. But I don't think that means that deserialization failure is a good way to communicate that to users. I can think of examples in Apple's frameworks where a method call returns an optional even if a value of nil means that your program has no way to continue and must fail. Getting a system device in Metal, for instance. This suggests changing all properties to optionals. I'm curious on your thoughts here, because it feels a little extreme.

I see the other option being just making the change for headers along with a suggestion to users that their functions accept a String and coerce it with guard let into the request object if the requirement is really to ensure that the user's own lambda code runs and the workflow doesn't fail in the Lambda Runtime code.

@sebsto
Copy link
Contributor

sebsto commented Nov 15, 2024

I just checked the new runtime (v2, on main branch) and the error message in case of decoding failure is much more explicit than what we used to have with v1 :

{"errorType":"FunctionError","errorMessage":"keyNotFound(CodingKeys(stringValue: \"age\", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: \"No value associated with key CodingKeys(stringValue: \\\"age\\\", intValue: nil) (\\\"age\\\").\", underlyingError: nil))"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
2 participants