Api gateway response parsing #1903

gcbeattyAWS · 2024-12-10T19:53:06Z

Issue #, if available:
DOTNET-7839

Description of changes:

Converts api gateway response to httpresonse

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

normj

API Gateway sets common headers on the response. Like a request id. We should mimic those as much as possible.

normj · 2024-12-11T01:37:03Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+    /// </summary>
+    /// <param name="apiResponse">The API Gateway proxy response to convert.</param>
+    /// <returns>An HttpResponse object representing the API Gateway response.</returns>
+    public static HttpResponse ToHttpResponse(this APIGatewayProxyResponse apiResponse)


At some point I think here we need to do a size check that we aren't returning more then 6MB. And if there is an attempt return back the same error that API Gateway would do.

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

normj · 2024-12-11T01:41:23Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+        {
+            foreach (var header in headers)
+            {
+                response.Headers[header.Key] = header.Value;


We could be dealing with a IDictionary<string, string> collection that has comma delimited multi value headers. That needs to be handled here. I think this code will create a single header in API Gateway with the comma delimited string.

lambda code

return { statusCode: 200, headers: { 'Content-Type': 'application/json', 'myheader': 'test,other', 'anotherheader': 'firstvalue', 'anotherheader': 'secondvalue' }, multiValueHeaders: { "headername": ["headervalue", "headervalue2"] }, body: JSON.stringify({ event }), }; };

V1 Response headers

Myheader: test,shouldhavesecondvalue Anotherheader: secondvalue Headername: headervalue Headername: headervalue2

v2 response headers

Anotherheader: secondvalue Myheader: test,shouldhavesecondvalue

however, it does look like im missing one case, where headers and multivalueheaders are both defined for the same key, it should give me the combined result of all of them for v1.

i will add a test case to verify

normj · 2024-12-11T01:42:56Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+
+        response.StatusCode = apiResponse.StatusCode;
+
+        SetResponseHeaders(response, apiResponse.Headers, apiResponse.MultiValueHeaders);


I'm pretty sure if Content-Type is not returned by the Lambda function API Gateway will default to application/json. It might also set the content-length. We need to mimic that behavior but confirm my assumption.

i updated to set content length. for the default content-type, theres actually a couple of cases for how that can get set. ive written test cases for those and it may be easier to read them than me typing :D

so i dug more into this. this was tripping me up for a couple of hours. the default content type actually depends on what kind of api gateway is being used. If content-type is not specified and a REST api is being used, the content type is application/.json. but if its an http api being used (for v1) its text/plain. but then a http (v2) can sometimes infer the application type is json...

normj · 2024-12-11T01:43:12Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+
+        response.StatusCode = apiResponse.StatusCode;
+
+        SetResponseHeaders(response, apiResponse.Headers);


I'm pretty sure if Content-Type is not returned by the Lambda function API Gateway will default to application/json. It might also set the content-length. We need to mimic that behavior but confirm my assumption.

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayTestHelper.cs

...ol-v2/tests/Amazon.Lambda.TestTool.UnitTests/Extensions/ApiGatewayResponseExtensionsTests.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

...v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsSpecialCases.cs

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayTestHelper.cs

normj · 2024-12-16T00:24:49Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+    /// <param name="headers">The single-value headers to set.</param>
+    /// <param name="multiValueHeaders">The multi-value headers to set.</param>
+    /// <param name="emulatorMode">The API Gateway emulator mode determining which default headers to include.</param>
+    private static void SetResponseHeaders(HttpResponse response, IDictionary<string, string>? headers, IDictionary<string, IList<string>>? multiValueHeaders = null, ApiGatewayEmulatorMode emulatorMode = ApiGatewayEmulatorMode.HttpV2)


Having a default for emulatorMode seems odd to me. Defaulting the value to HttpV2 seems random.

fixed in latest revision

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

normj · 2024-12-16T00:34:31Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+                var errorBytes = Encoding.UTF8.GetBytes("{\"message\": \"Internal server error\"}");
+                response.Body = new MemoryStream(errorBytes);
+                response.ContentLength = errorBytes.Length;
+            } else


nit: Please put the else on a separate line.

fixed in latest revision

normj · 2024-12-16T00:38:58Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+        {
+            // API Gateway 2.0 format version assumptions
+            response.StatusCode = 200;
+            response.ContentType = "application/json";


Should you check to see if content type was already set from the headers collection coming back from the Lambda function?

ill add an integration test case to confirm what happens in that scenario and update accordingly

ive added V2_TestJsonInfersAlwaysUsesApplicationJson. i believe my current logic is right. the reason being is for this scenario, since status code is 0, api gateway just uses the whole payload as the body response and doesnt consider any lambda provided headers.

i updated this logic to just be straightforward - with the results from my manual testing. that if status code is not set, then api gateway http 2.0 will default to application/json

i guess what you meant by this (based on your other comment with the binary type for rest api), is that if the user configured custom response headers for this error code that it might be in the response already? ill have to check that

edit: actually i guess my above crossed out comment only applies to the other code up above where i do internal server error. so ill see what happens in that case. i think the user can specify custom response headers in rest api depending on certain error codes

so i was thinking about this more with the way the v2 infers things. i think we need another layer before this api gateway response -> http response. i think we need a lambda memory stream -> api gateway response, which will do these inferences

i removed the json inference check from here. i think we should add the lambda -> api gateay translation inference in another pr

...2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsJsonInference.cs

normj · 2024-12-16T00:44:44Z

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayTestHelper.cs

+            _httpClient = new HttpClient();
+        }
+
+        public async Task<string> CreateLambdaFunctionAsync(string roleArn, string lambdaCode)


Please create the function with a CloudFormation template so it is easier to tear down when we have orphan test data when tests crash.

~~how would a cloud formation template for the lambda be easier to tear down for the lambda exactly?~~ edit: i talked with phil. since it would all be bundled together it would be easier to just delete the stack

i would like to update it to use cloudformation in a separate PR, after i get the main functionality in.

actually, i figured out how to do this. i will push an update using cloudformation

ive updated it to use cloudformation to create everything. i reran the integration tests too and they all pass (except for the 1 i have intentionally failing right now related to the v2 json assumptions)

...ol-v2/tests/Amazon.Lambda.TestTool.UnitTests/Extensions/ApiGatewayResponseExtensionsTests.cs

gcbeattyAWS · 2024-12-17T06:49:51Z

@normj we can discuss more on 12/18 on the application/json default logic when the status code is 0 for http v2. i feel that is the last big thing functionality wise missing. so far i have been able to test when the status code is 0 and the body is valid json. but i have not been able to figure out how to verify what happens when the status code is 0 and the body is not valid json

Tools/LambdaTestTool-v2/AWS.Lambda.TestTool.slnx

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ServiceCollectionExtensions.cs

...tests/Amazon.Lambda.TestTool.IntegrationTests/Amazon.Lambda.TestTool.IntegrationTests.csproj

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ServiceCollectionExtensions.cs

...2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsJsonInference.cs

normj · 2024-12-18T18:20:39Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs

+        if (!string.IsNullOrEmpty(body))
+        {
+            byte[] bodyBytes;
+            if (isBase64Encoded && ApiGatewayEmulatorMode.Rest != apiGatewayEmulator) // rest api gateway doesnt automatically decode the response


If isBase64Encoded is true we should always decode the body. I think what you are seeing is that in API Gateway you need to configure your REST API for binary media. And then it will decode it. From our emulator's point of view we should assume that they have correctly configured binary media and so we should decode base 64 encoded bodies.

In CloudFormation you can configure binary media by adding the following resource:

"TheApi": { "Type": "AWS::Serverless::Api", "Properties": { "StageName": "Prod", "BinaryMediaTypes": [ "*~1*" ] } },

Then on the event configuration for the serviceless function reference the API:

], "PackageType": "Zip", "Events": { "RootGet": { "Type": "Api", "Properties": { "Path": "/", "Method": "GET", "RestApiId" : { "Ref" : "TheApi" } } } ``

i see. that certainly makes more sense. ill test it out to see what happens

yeah i just tested this and you are right. i will update this logic and add a test case

ive fixed this now and have also added an integration and unit test

normj · 2024-12-18T18:29:59Z

...-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsTestsManual.cs

+namespace Amazon.Lambda.TestTool.IntegrationTests
+{
+    [Collection("ApiGateway Integration Tests")]
+    public class ApiGatewayResponseExtensionsTestsManual


What does the Manual suffix mean here? This test will run automatically as part of integration test runs.

ill rename this to something better. its not part of the ApiGatewayResponeTestCases list and the test cases are separate. maybe AdditionalTests

gcbeattyAWS · 2024-12-18T20:37:09Z

...stTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsTests.cs

+
+        [Theory]
+        [MemberData(nameof(ApiGatewayResponseTestCases.V1TestCases), MemberType = typeof(ApiGatewayResponseTestCases))]
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "xUnit1026:Theory methods should use all of their parameters")]


i added this suppression so in vscode we can easily see what test is being executed by having test name parameter here

gcbeattyAWS · 2024-12-19T21:59:40Z

...tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsAdditionalTests.cs

+        }
+
+        //[Fact]
+        //public async Task V2_SetsContentTypeApplicationJsonWhenNoStatusProvided()


i commented this out for now so it can be referenced for future when we create the lambda -> api gateway response layer

gcbeattyAWS marked this pull request as ready for review December 10, 2024 20:10

gcbeattyAWS requested a review from philasmar December 10, 2024 20:10

gcbeattyAWS changed the base branch from asmarp/api-gateway-emulator-skeleton to feature/lambdatesttool-v2 December 10, 2024 20:16

gcbeattyAWS changed the base branch from feature/lambdatesttool-v2 to asmarp/api-gateway-emulator-skeleton December 10, 2024 20:17

gcbeattyAWS requested a review from normj December 10, 2024 20:21

gcbeattyAWS added the Release Not Needed Add this label if a PR does not need to be released. label Dec 10, 2024

normj reviewed Dec 11, 2024

View reviewed changes

gcbeattyAWS force-pushed the gcbeatty/responseparsing branch 2 times, most recently from 56af10b to 8d33c54 Compare December 13, 2024 22:44

gcbeattyAWS commented Dec 13, 2024

View reviewed changes

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayTestHelper.cs Outdated Show resolved Hide resolved

gcbeattyAWS commented Dec 13, 2024

View reviewed changes

...ol-v2/tests/Amazon.Lambda.TestTool.UnitTests/Extensions/ApiGatewayResponseExtensionsTests.cs Outdated Show resolved Hide resolved

gcbeattyAWS commented Dec 13, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ApiGatewayResponseExtensions.cs Outdated Show resolved Hide resolved

gcbeattyAWS commented Dec 13, 2024

View reviewed changes

...v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsSpecialCases.cs Outdated Show resolved Hide resolved

gcbeattyAWS commented Dec 13, 2024

View reviewed changes

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayTestHelper.cs Outdated Show resolved Hide resolved

gcbeattyAWS force-pushed the gcbeatty/responseparsing branch 2 times, most recently from 090e360 to 22eb253 Compare December 14, 2024 06:21

normj reviewed Dec 16, 2024

View reviewed changes

gcbeattyAWS force-pushed the gcbeatty/responseparsing branch from 22eb253 to d0707ad Compare December 17, 2024 05:58

gcbeattyAWS changed the base branch from asmarp/api-gateway-emulator-skeleton to feature/lambdatesttool-v2 December 17, 2024 05:58

gcbeattyAWS commented Dec 17, 2024

View reviewed changes

Tools/LambdaTestTool-v2/AWS.Lambda.TestTool.slnx Outdated Show resolved Hide resolved

philasmar requested changes Dec 17, 2024

View reviewed changes

gcbeattyAWS commented Dec 18, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ServiceCollectionExtensions.cs Outdated Show resolved Hide resolved

gcbeattyAWS commented Dec 18, 2024

View reviewed changes

...2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayResponseExtensionsJsonInference.cs Outdated Show resolved Hide resolved

Add api gateway extensions

8cfbc0a

gcbeattyAWS force-pushed the gcbeatty/responseparsing branch 2 times, most recently from 2887025 to a374bd5 Compare December 18, 2024 16:06

gcbeattyAWS requested review from philasmar and normj December 18, 2024 16:06

normj reviewed Dec 18, 2024

View reviewed changes

First set of PR comments

b4cd801

gcbeattyAWS commented Dec 18, 2024

View reviewed changes

PR comments

7e88575

gcbeattyAWS force-pushed the gcbeatty/responseparsing branch from 9451d3c to 7e88575 Compare December 19, 2024 00:45

PR comments

66a0c9c

gcbeattyAWS commented Dec 19, 2024

View reviewed changes

normj approved these changes Dec 20, 2024

View reviewed changes

philasmar approved these changes Dec 20, 2024

View reviewed changes

gcbeattyAWS merged commit 5c1d3ef into feature/lambdatesttool-v2 Dec 20, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api gateway response parsing #1903

Api gateway response parsing #1903

gcbeattyAWS commented Dec 10, 2024 •

edited

Loading

normj left a comment

normj Dec 11, 2024

normj Dec 11, 2024

gcbeattyAWS Dec 12, 2024 •

edited

Loading

gcbeattyAWS Dec 12, 2024

normj Dec 11, 2024

gcbeattyAWS Dec 12, 2024

gcbeattyAWS Dec 13, 2024

normj Dec 11, 2024

normj Dec 16, 2024

gcbeattyAWS Dec 17, 2024

normj Dec 16, 2024

gcbeattyAWS Dec 17, 2024

normj Dec 16, 2024

gcbeattyAWS Dec 16, 2024

gcbeattyAWS Dec 17, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS Dec 18, 2024 •

edited

Loading

gcbeattyAWS Dec 19, 2024 •

edited

Loading

gcbeattyAWS Dec 19, 2024

normj Dec 16, 2024

gcbeattyAWS Dec 16, 2024 •

edited

Loading

gcbeattyAWS Dec 16, 2024

gcbeattyAWS Dec 17, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS commented Dec 17, 2024

normj Dec 18, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS Dec 19, 2024

normj Dec 18, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS Dec 18, 2024

gcbeattyAWS Dec 19, 2024


		response.StatusCode = apiResponse.StatusCode;

		SetResponseHeaders(response, apiResponse.Headers, apiResponse.MultiValueHeaders);

Api gateway response parsing #1903

Api gateway response parsing #1903

Conversation

gcbeattyAWS commented Dec 10, 2024 • edited Loading

normj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

gcbeattyAWS Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS commented Dec 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS commented Dec 10, 2024 •

edited

Loading

gcbeattyAWS Dec 12, 2024 •

edited

Loading

gcbeattyAWS Dec 18, 2024 •

edited

Loading

gcbeattyAWS Dec 19, 2024 •

edited

Loading

gcbeattyAWS Dec 16, 2024 •

edited

Loading