Skip to content

Commit

Permalink
Handle invalid field errors in Mango
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
nickva committed Jul 30, 2023
1 parent 1f2994e commit 64af302
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 3 deletions.
8 changes: 6 additions & 2 deletions src/fabric/src/fabric_view.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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()}].
Expand Down
7 changes: 6 additions & 1 deletion src/mango/src/mango_cursor_view.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/mango/src/mango_error.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions src/mango/src/mango_httpd.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/mango/test/02-basic-find-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def test_bad_selector(self):
{"foo": {"$not_an_op": 2}},
{"$gt": 2},
[None, "bing"],
{"_id": {"": None}},
]
for bs in bad_selectors:
try:
Expand Down

0 comments on commit 64af302

Please sign in to comment.