Skip to content
New issue

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

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

Already on GitHub? # 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],[]}]}}]}

# for free to join this conversation on GitHub. Already have an account? # 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