Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all config get's throw on no ets table #4894

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

chewbranca
Copy link
Contributor

This fixes #4890 by ensuring that the type coercion get wrappers in config:get_* perform the call to config:get/3 outside of the try-catch block intended to catch type coercion exceptions.

@nickva
Copy link
Contributor

nickva commented Dec 4, 2023

For the failing tests we'd probably want to either mock config get or make sure config app is started.

@chewbranca
Copy link
Contributor Author

Hmmm... the failures in that build seem to be isolated to just chttpd_db for the most part, and the build only took 13 minutes, I suspect there will be many more failures. In the case here, I see we're already meck'ing config:get/3 https://github.com/apache/couchdb/blob/main/src/chttpd/src/chttpd_db.erl#L2612 which should work, unless meck only intercedes public invocations of the module, rather than an internal invocation from get_integer. You can see we're hitting {config_meck_original, get, 3} in the stacktrace despite that test being covered by the setup clause linked above mecking config:get/3:

18:15:58  chttpd_db:2687 -t_should_throw_on_invalid_n/0-fun-1-
18:15:58  ----------------------------------------------------
18:15:58  
18:15:58  ::in function chttpd_db:'-t_should_throw_on_invalid_n/0-fun-1-'/0 (src/chttpd_db.erl, line 2690)
18:15:58  in call from eunit_test:run_testfun/1 (eunit_test.erl, line 71)
18:15:58  in call from eunit_proc:run_test/1 (eunit_proc.erl, line 531)
18:15:58  in call from eunit_proc:with_timeout/3 (eunit_proc.erl, line 356)
18:15:58  in call from eunit_proc:handle_test/2 (eunit_proc.erl, line 514)
18:15:58  in call from eunit_proc:tests_inorder/3 (eunit_proc.erl, line 456)
18:15:58  in call from eunit_proc:with_timeout/3 (eunit_proc.erl, line 346)
18:15:58  in call from eunit_proc:run_group/2 (eunit_proc.erl, line 570)
18:15:58  **error:{assertException,
18:15:58      [{module,chttpd_db},
18:15:58       {line,2690},
18:15:58       {expression,"parse_shards_opt ( Req )"},
18:15:58       {pattern,"{ throw , { bad_request , Err } , [...] }"},
18:15:58       {unexpected_exception,
18:15:58           {error,badarg,
18:15:58               [{ets,lookup,
18:15:58                    [config,{"cluster","n"}],
18:15:58                    [{error_info,#{cause => id,module => erl_stdlib_errors}}]},
18:15:58                {config_meck_original,get,3,
18:15:58                    [{file,"src/config.erl"},{line,163}]},
18:15:58                {config_meck_original,get_integer,3,
18:15:58                    [{file,"src/config.erl"},{line,71}]},
18:15:58                {chttpd_db,parse_shards_opt,1,
18:15:58                    [{file,"src/chttpd_db.erl"},{line,1951}]},
18:15:58                {chttpd_db,'-t_should_throw_on_invalid_n/0-fun-1-',0,
18:15:58                    [{file,"src/chttpd_db.erl"},{line,2690}]},
18:15:58                {eunit_test,run_testfun,1,[{file,"eunit_test.erl"},{line,71}]},
18:15:58                {eunit_proc,run_test,1,[{file,"eunit_proc.erl"},{line,531}]},
18:15:58                {config,get_integer,["cluster","n",3],[]}]}}]}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent behavior between config:get* accessors during cache failures
3 participants