Handle disconnect route of Websocket #548

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

@@ -97,11 +97,11 @@
    "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
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle disconnect route of Websocket #548

Handle disconnect route of Websocket #548

kdnakt commented Jan 20, 2024

codecov-commenter commented Jan 20, 2024 •

edited

tinhda left a comment

bmoffatt Jan 24, 2024

kdnakt Jan 25, 2024

bmoffatt Jan 24, 2024

kdnakt Jan 25, 2024

bmoffatt Jan 24, 2024 •

edited

tinhda Jan 24, 2024 •

edited

kdnakt Jan 25, 2024

bmoffatt Jan 26, 2024

kdnakt Feb 10, 2024

kdnakt Mar 21, 2024

bmoffatt Mar 22, 2024

bmoffatt Jan 26, 2024

kdnakt Jan 28, 2024

bmoffatt left a comment

bmoffatt Mar 22, 2024

bmoffatt Mar 22, 2024

kdnakt Apr 7, 2024

kdnakt commented Mar 29, 2024

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

Handle disconnect route of Websocket #548

Handle disconnect route of Websocket #548

Conversation

kdnakt commented Jan 20, 2024

codecov-commenter commented Jan 20, 2024 • edited

Codecov Report

tinhda left a 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

bmoffatt Jan 24, 2024 • edited

Choose a reason for hiding this comment

tinhda Jan 24, 2024 • edited

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

bmoffatt left a 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

kdnakt commented Mar 29, 2024

codecov-commenter commented Jan 20, 2024 •

edited

bmoffatt Jan 24, 2024 •

edited

tinhda Jan 24, 2024 •

edited