Repository: marshmallow-code/webargs

Issue:
[RFC] Namespace error structures by location in response
I think there's a case we never really considered: two schemas in a different location with a common field name.

In webargs 5, this is not possible in a single `use_args` call. In a multi `use_args` call, it depends if the user specifies the locations. The response is ambiguous about which field is wrong.

In current implementation in webargs 6, the response is ambiguous. A way to fix this would be to namespace the error structures by location.

Just thinking out loud, writing this here before I forget.

Discussion:

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index c11fe33..6991c91 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,16 @@
 Changelog
 ---------
 
+6.0.0b7 (Unreleased)
+********************
+
+Features:
+
+* *Backwards-incompatible*: webargs will rewrite the error messages in
+  ValidationErrors to be namespaced under the location which raised the error.
+  The `messages` field on errors will therefore be one layer deeper with a
+  single top-level key.
+
 6.0.0b6 (2020-01-31)
 ********************
 
diff --git a/src/webargs/asyncparser.py b/src/webargs/asyncparser.py
index 0eb6037..ec9773b 100644
--- a/src/webargs/asyncparser.py
+++ b/src/webargs/asyncparser.py
@@ -36,6 +36,7 @@ class AsyncParser(core.Parser):
         Receives the same arguments as `webargs.core.Parser.parse`.
         """
         req = req if req is not None else self.get_default_request()
+        location = location or self.location
         if req is None:
             raise ValueError("Must pass req object")
         data = None
@@ -43,14 +44,14 @@ class AsyncParser(core.Parser):
         schema = self._get_schema(argmap, req)
         try:
             location_data = await self._load_location_data(
-                schema=schema, req=req, location=location or self.location
+                schema=schema, req=req, location=location
             )
             result = schema.load(location_data)
             data = result.data if core.MARSHMALLOW_VERSION_INFO[0] < 3 else result
             self._validate_arguments(data, validators)
         except ma.exceptions.ValidationError as error:
             await self._on_validation_error(
-                error, req, schema, error_status_code, error_headers
+                error, req, schema, location, error_status_code, error_headers
             )
         return data
 
@@ -78,9 +79,16 @@ class AsyncParser(core.Parser):
         error: ValidationError,
         req: Request,
         schema: Schema,
+        location: str,
         error_status_code: typing.Union[int, None],
         error_headers: typing.Union[typing.Mapping[str, str], None] = None,
     ) -> None:
+        # rewrite messages to be namespaced under the location which created
+        # them
+        # e.g. {"json":{"foo":["Not a valid integer."]}}
+        #      instead of
+        #      {"foo":["Not a valid integer."]}
+        error.messages = {location: error.messages}
         error_handler = self.error_callback or self.handle_error
         await error_handler(error, req, schema, error_status_code, error_headers)
 
diff --git a/src/webargs/core.py b/src/webargs/core.py
index b5fe2f9..8160b7c 100644
--- a/src/webargs/core.py
+++ b/src/webargs/core.py
@@ -168,8 +168,14 @@ class Parser:
         return data
 
     def _on_validation_error(
-        self, error, req, schema, error_status_code, error_headers
+        self, error, req, schema, location, error_status_code, error_headers
     ):
+        # rewrite messages to be namespaced under the location which created
+        # them
+        # e.g. {"json":{"foo":["Not a valid integer."]}}
+        #      instead of
+        #      {"foo":["Not a valid integer."]}
+        error.messages = {location: error.messages}
         error_handler = self.error_callback or self.handle_error
         error_handler(error, req, schema, error_status_code, error_headers)
 
@@ -234,6 +240,7 @@ class Parser:
          :return: A dictionary of parsed arguments
         """
         req = req if req is not None else self.get_default_request()
+        location = location or self.location
         if req is None:
             raise ValueError("Must pass req object")
         data = None
@@ -241,14 +248,14 @@ class Parser:
         schema = self._get_schema(argmap, req)
         try:
             location_data = self._load_location_data(
-                schema=schema, req=req, location=location or self.location
+                schema=schema, req=req, location=location
             )
             result = schema.load(location_data)
             data = result.data if MARSHMALLOW_VERSION_INFO[0] < 3 else result
             self._validate_arguments(data, validators)
         except ma.exceptions.ValidationError as error:
             self._on_validation_error(
-                error, req, schema, error_status_code, error_headers
+                error, req, schema, location, error_status_code, error_headers
             )
         return data
 


Test patch:
diff --git a/tests/apps/flask_app.py b/tests/apps/flask_app.py
index 17030f0..cfa6d12 100644
--- a/tests/apps/flask_app.py
+++ b/tests/apps/flask_app.py
@@ -212,4 +212,16 @@ def echo_use_kwargs_missing(username, **kwargs):
 def handle_error(err):
     if err.code == 422:
         assert isinstance(err.data["schema"], ma.Schema)
-    return J(err.data["messages"]), err.code
+
+    if MARSHMALLOW_VERSION_INFO[0] >= 3:
+        return J(err.data["messages"]), err.code
+
+    # on marshmallow2, validation errors for nested schemas can fail to encode:
+    # https://github.com/marshmallow-code/marshmallow/issues/493
+    # to workaround this, convert integer keys to strings
+    def tweak_data(value):
+        if not isinstance(value, dict):
+            return value
+        return {str(k): v for k, v in value.items()}
+
+    return J({k: tweak_data(v) for k, v in err.data["messages"].items()}), err.code
diff --git a/tests/test_core.py b/tests/test_core.py
index feef0c1..43df976 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -165,7 +165,9 @@ def test_parse_required_list(parser, web_request):
     args = {"foo": fields.List(fields.Field(), required=True)}
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(args, web_request)
-    assert excinfo.value.messages["foo"][0] == "Missing data for required field."
+    assert (
+        excinfo.value.messages["json"]["foo"][0] == "Missing data for required field."
+    )
 
 
 # Regression test for https://github.com/marshmallow-code/webargs/issues/107
@@ -180,7 +182,7 @@ def test_parse_list_dont_allow_none(parser, web_request):
     args = {"foo": fields.List(fields.Field(), allow_none=False)}
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(args, web_request)
-    assert excinfo.value.messages["foo"][0] == "Field may not be null."
+    assert excinfo.value.messages["json"]["foo"][0] == "Field may not be null."
 
 
 def test_parse_empty_list(parser, web_request):
@@ -369,7 +371,7 @@ def test_required_with_custom_error(parser, web_request):
         # Test that `validate` receives dictionary of args
         parser.parse(args, web_request)
 
-    assert "We need foo" in excinfo.value.messages["foo"]
+    assert "We need foo" in excinfo.value.messages["json"]["foo"]
     if MARSHMALLOW_VERSION_INFO[0] < 3:
         assert "foo" in excinfo.value.field_names
 
@@ -402,7 +404,7 @@ def test_full_input_validator_receives_nonascii_input(web_request):
     args = {"text": fields.Str()}
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(args, web_request, validate=validate)
-    assert excinfo.value.messages == ["Invalid value."]
+    assert excinfo.value.messages == {"json": ["Invalid value."]}
 
 
 def test_invalid_argument_for_validate(web_request, parser):
@@ -482,8 +484,9 @@ def test_parse_with_data_key_retains_field_name_in_error(web_request):
     args = {"content_type": fields.Str(**data_key_kwargs)}
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(args, web_request)
-    assert "Content-Type" in excinfo.value.messages
-    assert excinfo.value.messages["Content-Type"] == ["Not a valid string."]
+    assert "json" in excinfo.value.messages
+    assert "Content-Type" in excinfo.value.messages["json"]
+    assert excinfo.value.messages["json"]["Content-Type"] == ["Not a valid string."]
 
 
 def test_parse_nested_with_data_key(web_request):
@@ -607,7 +610,9 @@ def test_multiple_arg_required_with_int_conversion(web_request, parser):
     web_request.json = {}
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(args, web_request)
-    assert excinfo.value.messages == {"ids": ["Missing data for required field."]}
+    assert excinfo.value.messages == {
+        "json": {"ids": ["Missing data for required field."]}
+    }
 
 
 def test_parse_with_callable(web_request, parser):
@@ -888,7 +893,7 @@ def test_delimited_list_passed_invalid_type(web_request, parser):
 
     with pytest.raises(ValidationError) as excinfo:
         parser.parse(schema, web_request)
-    assert excinfo.value.messages == {"ids": ["Not a valid delimited list."]}
+    assert excinfo.value.messages == {"json": {"ids": ["Not a valid delimited list."]}}
 
 
 def test_missing_list_argument_not_in_parsed_result(web_request, parser):
diff --git a/tests/test_flaskparser.py b/tests/test_flaskparser.py
index f5f70e8..f8ccf36 100644
--- a/tests/test_flaskparser.py
+++ b/tests/test_flaskparser.py
@@ -28,7 +28,7 @@ class TestFlaskParser(CommonTestCase):
     def test_parsing_invalid_view_arg(self, testapp):
         res = testapp.get("/echo_view_arg/foo", expect_errors=True)
         assert res.status_code == 422
-        assert res.json == {"view_arg": ["Not a valid integer."]}
+        assert res.json == {"view_args": {"view_arg": ["Not a valid integer."]}}
 
     def test_use_args_with_view_args_parsing(self, testapp):
         res = testapp.get("/echo_view_arg_use_args/42")
@@ -87,7 +87,7 @@ def test_abort_called_on_validation_error(mock_abort):
     abort_args, abort_kwargs = mock_abort.call_args
     assert abort_args[0] == 422
     expected_msg = "Invalid value."
-    assert abort_kwargs["messages"]["value"] == [expected_msg]
+    assert abort_kwargs["messages"]["json"]["value"] == [expected_msg]
     assert type(abort_kwargs["exc"]) == ValidationError