fix: enable responses from result of lambda function #1064
Conversation
|
@kevbot-git I think for test we need this PR #814 |
|
I'm willing to merge, can you explain more what you tried to achieve? |
|
@dherault all I have done is here is attempted to more closely emulate the behaviour of AWS API Gateway WebSocket APIs, which send a response socket event when messages are received 😃 |
…1008-websocket-handler-responses
|
Hi, just wondering what the status of this PR is? Is something blocking it being merged or is still under review? There has not been any further work on it in a while and we need this feature to be merged so that our dev stack will work. We can work around it at the moment by patching serverless-offline but it would be nice to have this PR merged so that we can use serverless-offline without patching it. |
…feature/1008-websocket-handler-responses
|
@coulson84 sorry for the late reply. I haven't looked at this in months but I've just pulled master over it and was looking at my code... I think I had understood the problem incorrectly at the time. I have changed the code so that it checks for the definition of a default route before sending the default |
|
Hey @dherault, I reckon this PR is still good to include as it appears to make offline WebSockets more closely follow AWS Lambda's behaviour with default responses etc. I'm so happy to take criticism on the code but wondering what you reckon? |
|
I'm just on my phone at the moment so haven't looked at the code just now. I think it had already accounted for this in the PR but I think AWS doesn't return any response from the $connect route |
|
I was about to pester for this to be merged but it needs a quick tweak first. Any response from the $connect route is ignored in AWS so the call to websocketClient.send just needs to be wrapped in an if statement to prevent the body being sent in that condition. Other than that it should all be good afaik 👍 |
|
Hi can you rebase please? I'm back on the project and would love to merge this. |
dnalborczyk
force-pushed
the
master
branch
2 times, most recently
from
d5ea86d to
22fd667
Compare
dnalborczyk
force-pushed
the
master
branch
2 times, most recently
from
fdd1699 to
51a30e9
Compare
dnalborczyk
changed the title
|
superseded by: #1301 |
Implements feature request #1008, fixes #1008
Drafting as I'm not yet sure of the best way to test... There doesn't seem to be much coverage on this file in particular—should I maybe do a per-file test
WebSocketClients.test.js?