[couchdb] 01/01: Handle invalid field errors in Mango

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch mango-handle-invalid-field-error
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 17d80035eb3790e8e26764be06c1604ddc63981e
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Sat Jul 29 02:30:38 2023 -0400

    Handle invalid field errors in Mango
    
    Add handling for invalid field errors. Previously they would emit a 500
    `badmatch` error with a stack trace:
    
    ```
    % curl -v -XPOST -H'Content-type:application/json' $URL/db/_find -d '{"selector":{"_id":{"":null}}}'
    
    < HTTP/1.1 500 Internal Server Error
    
    {"error":"badmatch","reason":"{error,{{mango_error,mango_util,{invalid_field_name...
    ```
    
    With the fix it emits a 400 error:
    
    ```
    < HTTP/1.1 400 Bad Request
    
    {"error":"invalid_field_name","reason":"Invalid field name: _id."}
    ```
    
    This should also handle a general class of mango errors from `$db/_find` which
    are sent from the workers, passed through the fabric/rexi mechanism, and
    handled on the coordinator side. Previously some of those would emit the same
    500 badmatch + a stack trace since we didn't expect errors in a few places
    which generated a `badmatch`.
---
 src/fabric/src/fabric_view.erl       | 8 ++++++--
 src/mango/src/mango_cursor_view.erl  | 7 ++++++-
 src/mango/src/mango_error.erl        | 6 ++++++
 src/mango/src/mango_httpd.erl        | 5 +++++
 src/mango/test/02-basic-find-test.py | 1 +
 5 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/fabric/src/fabric_view.erl b/src/fabric/src/fabric_view.erl
index 82f9c2bc6..22535a9f9 100644
--- a/src/fabric/src/fabric_view.erl
+++ b/src/fabric/src/fabric_view.erl
@@ -51,8 +51,12 @@ check_down_shards(Collector, BadNode) ->
 -spec handle_worker_exit(#collector{}, #shard{}, any()) -> {error, any()}.
 handle_worker_exit(Collector, _Worker, Reason) ->
     #collector{callback = Callback, user_acc = Acc} = Collector,
-    {ok, Resp} = Callback({error, fabric_util:error_info(Reason)}, Acc),
-    {error, Resp}.
+    case Callback({error, fabric_util:error_info(Reason)}, Acc) of
+        {error, Resp} ->
+            {error, Resp};
+        {ok, Resp} ->
+            {error, Resp}
+    end.
 
 -spec remove_overlapping_shards(#shard{}, [{#shard{}, any()}]) ->
     [{#shard{}, any()}].
diff --git a/src/mango/src/mango_cursor_view.erl b/src/mango/src/mango_cursor_view.erl
index 690f87054..e881d569c 100644
--- a/src/mango/src/mango_cursor_view.erl
+++ b/src/mango/src/mango_cursor_view.erl
@@ -238,7 +238,7 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu
                         fabric:query_view(Db, DbOpts, DDoc, Name, CB, Cursor, Args)
                 end,
             case Result of
-                {ok, LastCursor} ->
+                {ok, #cursor{} = LastCursor} ->
                     NewBookmark = mango_json_bookmark:create(LastCursor),
                     Arg = {add_key, bookmark, NewBookmark},
                     {_Go, FinalUserAcc} = UserFun(Arg, LastCursor#cursor.user_acc),
@@ -252,6 +252,11 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu
                         UserFun, Cursor, Stats1, FinalUserAcc0
                     ),
                     {ok, FinalUserAcc1};
+                {ok, Error} when is_tuple(Error) ->
+                    % fabric_view_all_docs turns {error, Resp} results into {ok, Resp}
+                    % for some reason. If we didn't get a proper cursor record, assume
+                    % it's an error and pass it through
+                    {error, Error};
                 {error, Reason} ->
                     {error, Reason}
             end
diff --git a/src/mango/src/mango_error.erl b/src/mango/src/mango_error.erl
index a8b5bfa4b..f1c343854 100644
--- a/src/mango/src/mango_error.erl
+++ b/src/mango/src/mango_error.erl
@@ -400,6 +400,12 @@ info(mango_util, {invalid_ddoc_lang, Lang}) ->
         <<"invalid_ddoc_lang">>,
         fmt("Existing design doc has an invalid language: ~w", [Lang])
     };
+info(mango_util, {invalid_field_name, Field}) ->
+    {
+        400,
+        <<"invalid_field_name">>,
+        fmt("Invalid field name: ~s", [Field])
+    };
 info(Module, Reason) ->
     {
         500,
diff --git a/src/mango/src/mango_httpd.erl b/src/mango/src/mango_httpd.erl
index ed797c517..bd91652da 100644
--- a/src/mango/src/mango_httpd.erl
+++ b/src/mango/src/mango_httpd.erl
@@ -214,6 +214,11 @@ handle_find_req(#httpd{method = 'POST'} = Req, Db) ->
     case run_find(Resp0, Db, Sel, Opts) of
         {ok, AccOut} ->
             end_find_resp(AccOut);
+        {error, {{mango_error, Mod, Reason}, nil, [_ | _] = Stack}} ->
+            % Re-raise mango errors so we can deal with them at the top in
+            % handle_req/2, otherwise chttpd:error_info/1 doesn't know what do
+            % with them.
+            erlang:raise(throw, {mango_error, Mod, Reason}, Stack);
         {error, Error} ->
             chttpd:send_error(Req, Error)
     end;
diff --git a/src/mango/test/02-basic-find-test.py b/src/mango/test/02-basic-find-test.py
index f156ef374..fd23a9fb2 100644
--- a/src/mango/test/02-basic-find-test.py
+++ b/src/mango/test/02-basic-find-test.py
@@ -26,6 +26,7 @@ class BasicFindTests(mango.UserDocsTests):
             {"foo": {"$not_an_op": 2}},
             {"$gt": 2},
             [None, "bing"],
+            {"_id": {"": None}}
         ]
         for bs in bad_selectors:
             try: