Repository: NilCoalescing/djangochannelsrestframework (language: python)

Issue:
Batter handling of malformed WS message bodies.
**Is your feature request related to a problem? Please describe.**
When sending messages, if the expected data for a given action is not present at the moment the consumer closes, rather than responding with an `error` message.

**Describe the solution you'd like**
Response with an error json message when:

* malformed JSON is sent over the websocket connection
* the action keyword is missing
* the action method expects a kwarge that was not provided as part of the action json



Discussion / hints:
I think I will help you do this. Please write some notes about how you want it to be implemented
@jakiro2017 That would be great. 

> the action keyword is missing

might be best to check in `AsyncAPIConsumer.receive_json` update the pop to return None if the key is not there. Then in `handle_action` inside the `try` block check that it is not None if it is raise the `MethodNotAllowed` error.

> the action method expects a kwarge that was not provided as part of the action json

To produce a nice error message you could use 
```
from inspect import signature
sig =  signature(method)
```

then validate that all the args in the sig match up to args in the message.

this would could be place somere here:
https://github.com/hishnash/djangochannelsrestframework/blob/master/djangochannelsrestframework/consumers.py#L140 

Then you could raise a DRF 400 error.

> malformed JSON is sent over the websocket connection

This is a little more complex complex since Django Channels is doing the json decoding we need to move that try catch up one more level.

```python
def receive(self, text_data=None, bytes_data=None, **kwargs):
     try:
         super().receive(text_data=text_data, bytes_data, **kwargs)
    except ...
```

Here you need to capture the json decoding and instead return the proper DRF error response.

Fix patch:
diff --git a/djangochannelsrestframework/consumers.py b/djangochannelsrestframework/consumers.py
index 842d05f..0a583e9 100644
--- a/djangochannelsrestframework/consumers.py
+++ b/djangochannelsrestframework/consumers.py
@@ -7,6 +7,7 @@
 
 import logging
 
+
 # Get an instance of a logger
 logger = logging.getLogger(__name__)
 
@@ -22,6 +23,7 @@
 from djangochannelsrestframework.settings import api_settings
 from djangochannelsrestframework.permissions import BasePermission, WrappedDRFPermission
 from djangochannelsrestframework.scope_utils import request_from_scope, ensure_async
+from djangochannelsrestframework.exceptions import ActionMissingException
 
 
 class APIConsumerMetaclass(type):
@@ -140,7 +142,9 @@ async def check_permissions(self, action: str, **kwargs):
             ):
                 raise PermissionDenied()
 
-    async def handle_exception(self, exc: Exception, action: str, request_id):
+    async def handle_exception(
+        self, exc: Exception, action: typing.Optional[str], request_id
+    ):
         """
         Handle any exception that occurs, by sending an appropriate message
         """
@@ -159,6 +163,10 @@ async def handle_exception(self, exc: Exception, action: str, request_id):
                 request_id=request_id,
             )
         else:
+            logger.error(
+                f"Error when handling request: {request_id} from client for action: {action}",
+                exc_info=exc,
+            )
             raise exc
 
     def _format_errors(self, errors):
@@ -202,8 +210,20 @@ async def handle_action(self, action: str, request_id: str, **kwargs):
             await self.handle_exception(exc, action=action, request_id=request_id)
 
     async def receive_json(self, content: typing.Dict, **kwargs):
-        request_id = content.pop("request_id")
-        action, content = await self.get_action_name(content, **kwargs)
+        request_id = content.pop("request_id", None)
+        action: str
+        content: Dict
+        try:
+            action, content = await self.get_action_name(content, **kwargs)
+        except KeyError as e:
+            await self.handle_exception(
+                ActionMissingException(), action=None, request_id=request_id
+            )
+            return
+        except Exception as e:
+            await self.handle_exception(e, action=None, request_id=request_id)
+            return
+
         await self.handle_action(action, request_id=request_id, **content)
 
     async def get_action_name(
@@ -220,7 +240,12 @@ async def get_action_name(
         return (action, content)
 
     async def reply(
-        self, action: str, data=None, errors=None, status=200, request_id=None
+        self,
+        action: typing.Optional[str],
+        data=None,
+        errors=None,
+        status=200,
+        request_id=None,
     ):
         """
         Send a json response back to the client.
diff --git a/djangochannelsrestframework/exceptions.py b/djangochannelsrestframework/exceptions.py
new file mode 100644
index 0000000..44831cb
--- /dev/null
+++ b/djangochannelsrestframework/exceptions.py
@@ -0,0 +1,9 @@
+from rest_framework import status
+from rest_framework.exceptions import MethodNotAllowed, APIException
+from django.utils.translation import gettext_lazy as _
+
+
+class ActionMissingException(APIException):
+    status_code = status.HTTP_405_METHOD_NOT_ALLOWED
+    default_detail = _("Unable to find action in message body.")
+    default_code = "method_not_allowed"


Test patch:
diff --git a/tests/test_consumer.py b/tests/test_consumer.py
index 0d6818d..f059238 100644
--- a/tests/test_consumer.py
+++ b/tests/test_consumer.py
@@ -264,3 +264,36 @@ async def test_async_action(self, pk=None, **kwargs):
     }
 
     await communicator.disconnect()
+
+
+@pytest.mark.django_db(transaction=True)
+@pytest.mark.asyncio
+async def test_error_on_missing_action():
+
+    event = asyncio.Event()
+
+    class AConsumer(AsyncAPIConsumer):
+        @action()
+        async def test_action(self, pk=None, **kwargs):
+            return {}, 200
+
+    # Test a normal connection
+    communicator = WebsocketCommunicator(AConsumer(), "/testws/")
+
+    connected, _ = await communicator.connect()
+
+    assert connected
+
+    await communicator.send_json_to({"pk": 2, "request_id": 1})
+
+    response = await communicator.receive_json_from()
+
+    assert response == {
+        "errors": ["Unable to find action in message body."],
+        "data": None,
+        "action": None,
+        "response_status": 405,
+        "request_id": 1,
+    }
+
+    await communicator.disconnect()