Websocket support #674

computerpunc · 2019-05-28T12:49:47Z

AWS WebSocket support that includes:

Setting up a ws server on port+1, by default port 3001 (this 2 server implementation mimics the AWS one)
Routing connect and disconnect events to the relevant handlers.
Routing any named action to the relevant handler.
In case of no action, route to the default handler.
Send messages to clients by POSTing to localhost:3001/@connections/{connectionId}.
Send messages to clients by calling the API - new ApiGatewayManagementApi({ apiVersion, endpoint }).postToConnection({ConnectionId, Data}).
Various error handling.
Backed by 13 end to end tests that run locally as well as on AWS.

I haven't tested everything, e.g., auth and https/wss.
I haven't yet updated the main README file.

Usage Assumption - In order to send messages back to clients

POST http://localhost:3001/@connections/{connectionId}

Or,

const endpoint=event.requestContext.domainName+'/'+event.requestContext.stage;

const apiVersion='2018-11-29';

let API=null;

if (!process.env.IS_OFFLINE) {

API = require('aws-sdk');

require('aws-sdk/clients/apigatewaymanagementapi');

} else {

API = require('serverless-offline').AWS;

}

new API.ApiGatewayManagementApi({ apiVersion, endpoint });

* Fix bug of test stop running when sending a an update to a non existing connection. * Fix but when querry string support was not equally handled as in AWS.

POST to /@connections/{connectionId} to send message to client at connectionId.

Here's the new way: const endpoint=event.requestContext.domainName+'/'+event.requestContext.stage; const apiVersion='2018-11-29'; let API=null; if (!process.env.IS_OFFLINE) { API = require('aws-sdk'); require('aws-sdk/clients/apigatewaymanagementapi'); } else { API = require('serverless-offline').AWS; } return new API.ApiGatewayManagementApi({ apiVersion, endpoint });

1. Action of routes. 2. Listening on WS message sending via REST API.

snamoah · 2019-05-28T13:53:19Z

@computerpunc the build is failing

dnalborczyk · 2019-05-28T14:52:37Z

static properties were introduced by one of the newer V8s available only in node v12+: https://github.com/dherault/serverless-offline/pull/674/files#diff-1fdf421c05c1140f6d71444ea2b27638R169

manual_test_websocket/README.md

manual_test_websocket/handler.js

src/index.js

dnalborczyk · 2019-05-28T18:46:16Z

@computerpunc just gave it a quick try. great job!!

one thing I noticed is that the context object appears to be empty: https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-websocket-api-mapping-template-reference.html

it seems to be working in general, but haven't looked or tried everything yet. partially also because of the empty context object.

edit: actually, the context object is empty, but I meant more so the requestContext on events. currently only includes: domainName, stage, connectionId. events itself includes body and requestContext.

src/index.js

Doesn't seem to be needed anymore as a workaround in aws-sdk.

Use plain vanilla AWS SDK: let endpoint=event.apiGatewayUrl; if (!endpoint) endpoint = event.requestContext.domainName+'/'+event.requestContext.stage; const apiVersion='2018-11-29'; return new AWS.ApiGatewayManagementApi({ apiVersion, endpoint });

dl748 · 2019-05-31T05:04:17Z

question, either approving this or the Hapi upgrade will break the other PR. Is there a choice on which will go in, which will require changes for the other developer?

dherault · 2019-05-31T08:00:38Z

I think I'll merge the Hapi upgrade first

mataide · 2019-05-31T16:00:02Z

Hey guys, this implementation works with Authorizer?

  socketsConnection:
            description: "Sockets Connection"
            handler: api/sockets/connection/handler.default
            memorySize: 256
            timeout: 30
            events:
              - websocket:
                  route: $connect
                  authorizer: socketsAuthorizer
              - websocket:
                  route: $disconnect

dnalborczyk · 2019-05-31T19:35:31Z

I think I'll merge the Hapi upgrade first

@dherault I think it might be better, as I think this PR still needs a bit of work. I wanna look into the hapi v18 merge conflict later again -- I have 1 or 2 test cases are failing.

I also looked through the code and did some incremental/fairly small changes, to get a better understanding, but also to reduce some of the complexity. I'll do a PR with it later, and then try to rebase v18 on top of it, if possible -- unless you are already on it.

Afterwards I wanna look closer into this PR.

@mataide I don't think so.

dherault · 2019-06-05T20:42:30Z

I reviewed it too and am willing to merge. The comments @dnalborczyk gave should be addressed first. Can't wait to use it for my next serverless project, it requires websockets!

dnalborczyk · 2019-06-05T20:42:54Z

(@dherault, JUST saw your comment 😃 )

@dherault @computerpunc

I pulled your PR into a branch: https://github.com/dherault/serverless-offline/tree/websockets

I think it's probably better and also easier to collaborate in the branch.

on first sight it doesn't seem to have any effect on any of the other code, other than sharing the hapi-server -- correct me if I'm wrong @computerpunc -- and therefore I'm thinking we could probably release it fairly soon, under some This is experimental functionality. Please report any bugs or missing features. banner, and then we do bugfixes and additions while using it and get feedback.

what do you guys think?

dherault · 2019-06-05T20:43:41Z

Agreed!

dnalborczyk · 2019-06-05T20:47:04Z

Can't wait to use it for my next serverless project, it requires websockets!

I'm using websockets for my current project as well. although, development is quite cumbersome. But as it seems not for long!

dherault · 2019-06-05T20:52:06Z

Please update the README as well. You're almost there @computerpunc!

computerpunc · 2019-06-05T21:57:19Z

createContext should be used from the already existing lambda context.

Using the existing lambda context is the way to go but at the current moment, I went with a solution that is decoupled. Most if not all of the code that could have been shared is not shared but copy/paste. I didn't want to accidentally degrade the HTTP part of the code. This in not only true for the present state but also for the future when I'll need to fix future bugs. I suggest that for v5.1, we can think of some kind of refactoring the HTTP and WS parts in which we'll use shared code for lambda context, server startup, etc. for both parts.

all the createXXX methods should probably go to their own file, as index.js is already quite ginormous

No problem. I'll put them in a separate file.

the lambda function handlers would need to be implemented I believe (promise, callback, context methods)

promise and callback are already handled (callback just recently) but context method is not recommended anymore by AWS and I'm not sure it's even supported in nodejs 8.10 runtime.

I believe the "action" property (or route) should not be assumed the way it is now. although $request.body.action is the default value for the route selection expression, if action is given, but the route doesn't exist, it errors out. I don't think aws does that (I might be wrong on that tho, have to double check). we should take websocketsApiRouteSelectionExpression into account.

OK. I'll take a look into it.

Please update the README as well. You're almost there @computerpunc!

Sure :)

dnalborczyk · 2019-06-06T03:55:57Z

@computerpunc awesome, thanks! I'm really looking forward for having this released!

I went with a solution that is decoupled.

makes sense.

promise and callback are already handled (callback just recently) but context method is not recommended anymore by AWS and I'm not sure it's even supported in nodejs 8.10 runtime.

that's true, it's doc-depreacted. I had no clue either, believe me. I did quite some research recently on that topic. good thing is it behaves pretty much identically as the callback. I probably submit some links for future references.

I'm planning on doing a couple PRs against the websockets branch as well in the next days ...

dherault · 2019-06-06T08:43:13Z

Maybe we should reopen this PR against the branch websockets ?

computerpunc · 2019-06-10T15:23:24Z

Hey guys, this implementation works with Authorizer?

I don't think so (in the respect that I don't have a test for it). I'll work on it after I finish working on injecting context and event to handlers.

I tried to add authorizer support but I don't think I'll be able to as there's a bug with authorization scheme and strategy in hapi-plugin-websocket in which it first opens a connection and only then sends checks for auth. I can write some sort of code to sync things up but it's a lot of work and it may be easier to fix it at the source (hapi-plugin-websocket).

And now to implementing websocketsApiRouteSelectionExpression.

In the form of $request.body.x.y.z

… into websocket-support

computerpunc · 2019-06-11T22:01:35Z

Guys,
No more things on my list.
Merge?

dnalborczyk · 2019-06-11T23:04:50Z

Guys,No more things on my list. Merge?

incredible job @computerpunc !!!

Let me pull this PR into the websockets branch. maybe we can polish it just a bit. that being said, I'm down with merging into master as well. I also think we should release soon! can't wait for having websocket support!

dnalborczyk · 2019-06-17T15:19:49Z

@computerpunc @dherault

I played with the websocket branch over the weekend a bit and added some small changes:

d802dcf collision resistent Ids with cuid, the former implementation is too risky to rely on, even with local development collisions would be expected. this will also benefit current HTTP requests.
8a699ab added a wsPort option. default value is the same: 3001. but the port will be not +1 anymore, it will be either default or as specified. I think that makes usage less confusing. [can debate about the naming.]
1386954 warning for experimental support. activates only when a websocket event is specified. can be removed after some time when released.
split ApiGateway and ApiGatewayWebSocket: 193e282 and 536d1b3 this looks like a lot, but it's actually not so much. it definitely needs some closer attention. @dherault if you could have a look. index.js just got too big, even before the websocket PR it was hard to know what was really going on. I spend most of the time in the code scrolling 😃 this is also something to be build upon, nothing set in stone. there's a lot of functionality to be reused and/or reduced. e.g. event creation etc. I also didn't rename private (underscore) function names yet, as those will likely anyways change over time. also:

this.apiGatewayWebSocket._createWsAction(fun, funName, servicePath, funOptions, event);
this.apiGateway._createRoutes(event, funOptions, protectedRoutes, funName, servicePath, serviceRuntime, defaultContentType, key, fun);

is more or less temporary, and can be greatly reduced and cleaned up over time.

also played a bit with websocketsApiRouteSelectionExpression, which seems to work as intended.

there's a bunch of other stuff missing for a full blown "offline" websocket simulation, but I think that's something which can be extended over time when the need arises.

I also have other additions and changes in stashes all over the place, but I'd rather have this PR released as well before adding/changing anything more.

computerpunc · 2019-06-17T16:21:23Z

I also have other additions and changes in stashes all over the place, but I'd rather have this PR released as well before adding/changing anything more.

Looking forward to see this PR released. In the meanwhile, I'm working on supporting authorizer in WS. I intent to release it after this PR is released.

dherault · 2019-06-17T18:14:13Z

I need some time to review this. @dnalborczyk should we merge this yet ? If we both review green I merge.

dnalborczyk · 2019-06-17T18:42:08Z

@dherault I added the changes in a branch here: https://github.com/dherault/serverless-offline/commits/websocket-fixes which includes this PR plus the above mentioned changes.

do you want me to open a new PR? or do you want to merge yourself locally?

dherault · 2019-06-18T07:57:08Z

Let's open a new PR!

dnalborczyk · 2019-06-18T16:11:23Z

closing in favor of #711

no worries @computerpunc , your work is still in the other PR (every single commit).

computerpunc added 11 commits May 9, 2019 00:40

demo websoket version

c4811ca

Added support for queryStringParameters

e32b9fa

Bug fixes

1ea68e0

* Fix bug of test stop running when sending a an update to a non existing connection. * Fix but when querry string support was not equally handled as in AWS.

Run WebSocket endpoint on a different port (http port+1)

53e9eef

GWAPI REST API suppoprt

602ac7b

POST to /@connections/{connectionId} to send message to client at connectionId.

Default timeout is 1000ms and can set timeout value via --timeout=

10c377d

Minor Improved logs

bca81cd

1. Action of routes. 2. Listening on WS message sending via REST API.

Update README with new ways of sending messages to clients

da8c749

Merge from upstream/master

fe4a51d

Leftovers from merge

f527670

Fix lint errors

4ec82ad

computerpunc added 2 commits May 28, 2019 18:10

Fix static AWS = { ... }

4e61aa5

Fix 2 lint errors

289e8f5

dnalborczyk reviewed May 28, 2019

View reviewed changes

manual_test_websocket/README.md Outdated Show resolved Hide resolved

dnalborczyk reviewed May 28, 2019

View reviewed changes

manual_test_websocket/handler.js Outdated Show resolved Hide resolved

dnalborczyk reviewed May 28, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

dnalborczyk reviewed May 28, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

dnalborczyk mentioned this pull request May 29, 2019

couple thoughts on serverless-offline, hapi update and websockets support #675

Closed

6 tasks

computerpunc added 3 commits May 29, 2019 08:18

Removed require('aws-sdk/clients/apigatewaymanagementapi');

ca34ccb

Doesn't seem to be needed anymore as a workaround in aws-sdk.

Last commit leftovers

f9600f8

Added context and event when calling the handler.

f3cf815

Moved websocket CreateXXX to websocketHelpers.js

381355b

computerpunc added 2 commits June 6, 2019 22:12

Restructure manual_test_websocket to include more projects

d45ea7a

Added serverless.yml with warning

66ffe6a

computerpunc added 6 commits June 11, 2019 23:19

Support for websocketsApiRouteSelectionExpression

02ce0e7

In the form of $request.body.x.y.z

Fix lint errors

3759b06

Merge branch 'master' of https://github.com/dherault/serverless-offline…

72a1fc4

… into websocket-support

Merge leftovers

687f901

Update README

b9b0eef

Update README about WebSocket

b6c1095

dnalborczyk mentioned this pull request Jun 18, 2019

Websocket #711

Merged

6 tasks

dnalborczyk closed this Jun 18, 2019

This was referenced Sep 12, 2020

[Snyk] Security upgrade node-fetch from 2.6.0 to 2.6.1 Itchibon777/serverless-offline#16

Open

[Snyk] Security upgrade node-fetch from 2.6.0 to 2.6.1 Itchibon777/serverless-offline#18

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Websocket support #674

Websocket support #674

computerpunc commented May 28, 2019

snamoah commented May 28, 2019

dnalborczyk commented May 28, 2019 •

edited

dnalborczyk commented May 28, 2019 •

edited

dl748 commented May 31, 2019 •

edited

dherault commented May 31, 2019

mataide commented May 31, 2019

dnalborczyk commented May 31, 2019 •

edited

dherault commented Jun 5, 2019

dnalborczyk commented Jun 5, 2019 •

edited

dherault commented Jun 5, 2019

dnalborczyk commented Jun 5, 2019

dherault commented Jun 5, 2019

computerpunc commented Jun 5, 2019

dnalborczyk commented Jun 6, 2019

dherault commented Jun 6, 2019

computerpunc commented Jun 10, 2019 •

edited

computerpunc commented Jun 11, 2019

dnalborczyk commented Jun 11, 2019

dnalborczyk commented Jun 17, 2019 •

edited

computerpunc commented Jun 17, 2019

dherault commented Jun 17, 2019

dnalborczyk commented Jun 17, 2019

dherault commented Jun 18, 2019

dnalborczyk commented Jun 18, 2019

Websocket support #674

Websocket support #674

Conversation

computerpunc commented May 28, 2019

Usage Assumption - In order to send messages back to clients

snamoah commented May 28, 2019

dnalborczyk commented May 28, 2019 • edited

dnalborczyk commented May 28, 2019 • edited

dl748 commented May 31, 2019 • edited

dherault commented May 31, 2019

mataide commented May 31, 2019

dnalborczyk commented May 31, 2019 • edited

dherault commented Jun 5, 2019

dnalborczyk commented Jun 5, 2019 • edited

dherault commented Jun 5, 2019

dnalborczyk commented Jun 5, 2019

dherault commented Jun 5, 2019

computerpunc commented Jun 5, 2019

dnalborczyk commented Jun 6, 2019

dherault commented Jun 6, 2019

computerpunc commented Jun 10, 2019 • edited

computerpunc commented Jun 11, 2019

dnalborczyk commented Jun 11, 2019

dnalborczyk commented Jun 17, 2019 • edited

computerpunc commented Jun 17, 2019

dherault commented Jun 17, 2019

dnalborczyk commented Jun 17, 2019

dherault commented Jun 18, 2019

dnalborczyk commented Jun 18, 2019

dnalborczyk commented May 28, 2019 •

edited

dnalborczyk commented May 28, 2019 •

edited

dl748 commented May 31, 2019 •

edited

dnalborczyk commented May 31, 2019 •

edited

dnalborczyk commented Jun 5, 2019 •

edited

computerpunc commented Jun 10, 2019 •

edited

dnalborczyk commented Jun 17, 2019 •

edited