Handle disconnect route of Websocket by kdnakt · Pull Request #548 · aws/aws-lambda-go

kdnakt · 2024-01-20T13:33:30Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov-commenter · 2024-01-20T13:35:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.30%. Comparing base (de51f68) to head (a8fbffb).

❗ Current head a8fbffb differs from pull request most recent head 9acd9d1. Consider uploading reports for the commit 9acd9d1 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #548   +/-   ##
=======================================
  Coverage   73.30%   73.30%           
=======================================
  Files          26       26           
  Lines        1487     1487           
=======================================
  Hits         1090     1090           
  Misses        324      324           
  Partials       73       73

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tinhda

thank you, just what I need!

bmoffatt · 2024-01-24T00:05:26Z

events/apigw.go

+	CognitoIdentityPoolID         string `json:"cognitoIdentityPoolId,omitempty"`
+	AccountID                     string `json:"accountId,omitempty"`
+	CognitoIdentityID             string `json:"cognitoIdentityId,omitempty"`
+	Caller                        string `json:"caller,omitempty"`
+	APIKey                        string `json:"apiKey,omitempty"`
+	APIKeyID                      string `json:"apiKeyId,omitempty"`
+	AccessKey                     string `json:"accessKey,omitempty"`
 	SourceIP                      string `json:"sourceIp"`
-	CognitoAuthenticationType     string `json:"cognitoAuthenticationType"`
-	CognitoAuthenticationProvider string `json:"cognitoAuthenticationProvider"`
-	UserArn                       string `json:"userArn"` //nolint: stylecheck
+	CognitoAuthenticationType     string `json:"cognitoAuthenticationType,omitempty"`
+	CognitoAuthenticationProvider string `json:"cognitoAuthenticationProvider,omitempty"`
+	UserArn                       string `json:"userArn,omitempty"` //nolint: stylecheck
 	UserAgent                     string `json:"userAgent"`
-	User                          string `json:"user"`
+	User                          string `json:"user,omitempty"`
 }

 // APIGatewayWebsocketProxyRequest contains data coming from the API Gateway proxy
 type APIGatewayWebsocketProxyRequest struct {
-	Resource                        string                                 `json:"resource"` // The resource path defined in API Gateway
-	Path                            string                                 `json:"path"`     // The url path for the caller
+	Resource                        string                                 `json:"resource,omitempty"` // The resource path defined in API Gateway
+	Path                            string                                 `json:"path,omitempty"`     // The url path for the caller
 	HTTPMethod                      string                                 `json:"httpMethod,omitempty"`
-	Headers                         map[string]string                      `json:"headers"`
-	MultiValueHeaders               map[string][]string                    `json:"multiValueHeaders"`
-	QueryStringParameters           map[string]string                      `json:"queryStringParameters"`
-	MultiValueQueryStringParameters map[string][]string                    `json:"multiValueQueryStringParameters"`
-	PathParameters                  map[string]string                      `json:"pathParameters"`
-	StageVariables                  map[string]string                      `json:"stageVariables"`
+	Headers                         map[string]string                      `json:"headers,omitempty"`
+	MultiValueHeaders               map[string][]string                    `json:"multiValueHeaders,omitempty"`
+	QueryStringParameters           map[string]string                      `json:"queryStringParameters,omitempty"`
+	MultiValueQueryStringParameters map[string][]string                    `json:"multiValueQueryStringParameters,omitempty"`
+	PathParameters                  map[string]string                      `json:"pathParameters,omitempty"`
+	StageVariables                  map[string]string                      `json:"stageVariables,omitempty"`
 	RequestContext                  APIGatewayWebsocketProxyRequestContext `json:"requestContext"`
-	Body                            string                                 `json:"body"`
-	IsBase64Encoded                 bool                                   `json:"isBase64Encoded,omitempty"`
+	Body                            string                                 `json:"body,omitempty"`
+	IsBase64Encoded                 *bool                                  `json:"isBase64Encoded,omitempty"`
 }

 // APIGatewayWebsocketProxyRequestContext contains the information to identify
 // the AWS account and resources invoking the Lambda function. It also includes
 // Cognito identity information for the caller.
 type APIGatewayWebsocketProxyRequestContext struct {
-	AccountID          string                    `json:"accountId"`
-	ResourceID         string                    `json:"resourceId"`
-	Stage              string                    `json:"stage"`
-	RequestID          string                    `json:"requestId"`
-	Identity           APIGatewayRequestIdentity `json:"identity"`
-	ResourcePath       string                    `json:"resourcePath"`
-	Authorizer         interface{}               `json:"authorizer"`
-	HTTPMethod         string                    `json:"httpMethod"`
-	APIID              string                    `json:"apiId"` // The API Gateway rest API Id
-	ConnectedAt        int64                     `json:"connectedAt"`
-	ConnectionID       string                    `json:"connectionId"`
-	DomainName         string                    `json:"domainName"`
-	Error              string                    `json:"error"`
-	EventType          string                    `json:"eventType"`
-	ExtendedRequestID  string                    `json:"extendedRequestId"`
-	IntegrationLatency string                    `json:"integrationLatency"`
-	MessageDirection   string                    `json:"messageDirection"`
-	MessageID          interface{}               `json:"messageId"`
-	RequestTime        string                    `json:"requestTime"`
-	RequestTimeEpoch   int64                     `json:"requestTimeEpoch"`
-	RouteKey           string                    `json:"routeKey"`
-	Status             string                    `json:"status"`
+	AccountID            string                    `json:"accountId,omitempty"`
+	ResourceID           string                    `json:"resourceId,omitempty"`
+	Stage                string                    `json:"stage"`
+	RequestID            string                    `json:"requestId"`
+	Identity             APIGatewayRequestIdentity `json:"identity"`
+	ResourcePath         string                    `json:"resourcePath,omitempty"`
+	Authorizer           interface{}               `json:"authorizer,omitempty"`
+	HTTPMethod           string                    `json:"httpMethod,omitempty"`
+	APIID                string                    `json:"apiId"` // The API Gateway rest API Id
+	ConnectedAt          int64                     `json:"connectedAt"`
+	ConnectionID         string                    `json:"connectionId"`
+	DomainName           string                    `json:"domainName"`
+	Error                string                    `json:"error,omitempty"`
+	EventType            string                    `json:"eventType"`
+	ExtendedRequestID    string                    `json:"extendedRequestId"`
+	IntegrationLatency   string                    `json:"integrationLatency,omitempty"`


undo the json tag changes, they're not related the the feature you're trying to add.

Thanks for your comment, but it's related to the feature because integrationLatency field is not in the WebSocket message sent on the $disconnect route as @tinhda wrote .
If I remove .omitempty json tag, TestApiGatewayWebsocketRequestDisconnectMarshaling would fail.

bmoffatt · 2024-01-24T00:05:43Z

events/apigw.go

+	ExtendedRequestID    string                    `json:"extendedRequestId"`
+	IntegrationLatency   string                    `json:"integrationLatency,omitempty"`
+	MessageDirection     string                    `json:"messageDirection"`
+	MessageID            *string                   `json:"messageId,omitempty"`


Changing the type of a struct field is a breaking change, revert it.

I reverted it in 5cdae93.

bmoffatt · 2024-01-24T00:19:04Z

events/apigw.go

+	DisconnectStatusCode int64                     `json:"disconnectStatusCode,omitempty"`
+	DisconnectReason     *string                   `json:"disconnectReason,omitempty"`


I couldn't find a reference to these fields in the AWS Docs, nor in the event libraries for .NET or Java.

Do you happen to have a reference handy? My search skills are failing me right now.

Hi, I found this sample online, you can also recreate this by looking into the websocket api gateway log in cloudwatch. I couldn't find anything in the aws documents neither.

Host: '3xcls223mg.execute-api.eu-west-1.amazonaws.com', 'x-api-key': '', 'X-Forwarded-For': '', 'x-restapi': '' }, multiValueHeaders: { Host: [ '3xcls223mg.execute-api.eu-west-1.amazonaws.com' ], 'x-api-key': [ '' ], 'X-Forwarded-For': [ '' ], 'x-restapi': [ '' ] }, requestContext: { routeKey: '$disconnect', disconnectStatusCode: 1006, eventType: 'DISCONNECT', extendedRequestId: 'fvLJ9EMQDoEFXSw=', requestTime: '22/May/2021:15:38:26 +0000', messageDirection: 'IN', disconnectReason: 'Connection Closed Abnormally', stage: 'updatedevice', connectedAt: 1621697605538, requestTimeEpoch: 1621697906882, identity: { userAgent: 'arduino-WebSocket-Client', sourceIp: '137.97.106.23' }, requestId: 'fvLJ9EMQDoEFXSw=', domainName: '3xcls223mg.execute-api.eu-west-1.amazonaws.com', connectionId: 'fvKa4d88joECFDw=', apiId: '3xcls223mg' }, isBase64Encoded: false }

Link to the sample: Links2004/arduinoWebSockets#667

Also found here: boto/boto3#3789

Unfortunately I'll need a reference that comes from docs.aws.amazon.com

I'll otherwise make time to walk myself through the developer guide so that I can reproduce the test payloads in this PR.

I found this developer guide updated!

https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-websocket-api-integration-requests.html#api-gateway-simple-proxy-for-lambda-input-format-websocket

Is there anything else I can do?

That's perfect! Thanks for following up!

bmoffatt · 2024-01-26T22:57:20Z

events/apigw.go

-	Body                            string                                 `json:"body"`
-	IsBase64Encoded                 bool                                   `json:"isBase64Encoded,omitempty"`
+	Body                            string                                 `json:"body,omitempty"`
+	IsBase64Encoded                 *bool                                  `json:"isBase64Encoded,omitempty"`


breaks back-compat, revert back to bool

Reverted the change. 🙇

bmoffatt

Another field type back-compat issue snuck in - revert it and the accomanying test data json, and this PR should be good to go

bmoffatt · 2024-03-22T23:01:07Z

events/apigw.go

+	ExtendedRequestID    string                    `json:"extendedRequestId"`
+	IntegrationLatency   string                    `json:"integrationLatency,omitempty"`
+	MessageDirection     string                    `json:"messageDirection"`
+	MessageID            string                    `json:"messageId,omitempty"`


MessageID needs to stay as interface{} for back-compat

bmoffatt · 2024-03-22T23:02:36Z

events/testdata/apigw-websocket-request.json

    "extendedRequestId": "TWegAcC4EowCHnA=",
    "integrationLatency": "123",
    "messageDirection": "IN",
-    "messageId": null,


"messageId": null should remain in the testdata

"messageId" field is not documented for CONNECT eventType.

https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-websocket-api-integration-requests.html#api-gateway-simple-proxy-for-lambda-input-format-websocket

And I couldn't find out a way to keep this field here in the testdata and also to keep the MessageID field of the APIGatewayWebsocketProxyRequestContext struct as interface{}.
If I can remove this field from the testdata, the MessageID field can stay as interface{}.

What should I do?

kdnakt · 2024-03-29T12:14:50Z

Thanks for your feedback, I'll fix the field type back-compat issue 🙇

Handle disconnect route of Websocket

a8d57a0

tinhda approved these changes Jan 22, 2024

View reviewed changes

bmoffatt requested changes Jan 24, 2024

View reviewed changes

bmoffatt reviewed Jan 24, 2024

View reviewed changes

bmoffatt added enhancement type/events issue or feature request related to the events package labels Jan 24, 2024

Revert MessageID type change

5cdae93

bmoffatt reviewed Jan 26, 2024

View reviewed changes

Revert field type change

a8fbffb

bmoffatt requested changes Mar 22, 2024

View reviewed changes

revert MessageID to interface{}

2da54a0

bmoffatt approved these changes Apr 16, 2024

View reviewed changes

Merge branch 'main' into apigateway-websocket

9acd9d1

bmoffatt merged commit 90a3af7 into aws:main Apr 16, 2024

		DisconnectStatusCode int64 `json:"disconnectStatusCode,omitempty"`
		DisconnectReason *string `json:"disconnectReason,omitempty"`

Conversation

kdnakt commented Jan 20, 2024

Uh oh!

codecov-commenter commented Jan 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tinhda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmoffatt Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tinhda Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmoffatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdnakt commented Mar 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jan 20, 2024 •

edited

Loading

bmoffatt Jan 24, 2024 •

edited

Loading

tinhda Jan 24, 2024 •

edited

Loading