The ESTermsQueryService.GetByName method throws APIErrorException when the term is not found.
In TermsController.GetByName, the fallback logic naively catches Exception as a "normal exception" and swallows it before trying to lookup the next combination.
In TermsControllerTests, the test cases GetByName_WithFallback_GeneticsHP and GetByName_WithFallback_TermsPatient set up mock instances of ITermsQueryService. The mocks include assertions of the order in which the various combinations are tried.
The assertions are incorrect with "off by one" errors (assuming the first callback to be number 1 instead of number 0), however the failures are swallowed by controller's logic and the failures are not reported. Because all exceptions are swallowed, no failure in checking the call order would be caught.
An "easy" fix would be to change the controller to explicitly catch APIErrorException, however this continues the use of "normal exceptions."
A better fix would be to refactor ESTermsQueryService.GetByName to return null when no term is found and modify the controller accordingly.
ESTIMATE 2
Steps to reproduce the issue
Open the API project in a debugger.
In TermsController.GetByName, set a breakpoint in the catch block of the fallback handling section of the code. (Lines 177-181 in the present code.)
In TermsControllerTest.GetByName.cs, debug the GetByName_WithFallback_GeneticsHP test and allow it to run.
What's the expected result?
In the current design, the catch block should be catching APIErrorException.
What's the actual result?
Xunit.Sdk.EqualException is caught and swallowed without being reported.
Issue description
The
ESTermsQueryService.GetByName
method throws APIErrorException when the term is not found.In
TermsController.GetByName
, the fallback logic naively catchesException
as a "normal exception" and swallows it before trying to lookup the next combination.In
TermsControllerTests
, the test casesGetByName_WithFallback_GeneticsHP
andGetByName_WithFallback_TermsPatient
set up mock instances ofITermsQueryService
. The mocks include assertions of the order in which the various combinations are tried.The assertions are incorrect with "off by one" errors (assuming the first callback to be number
1
instead of number0
), however the failures are swallowed by controller's logic and the failures are not reported. Because all exceptions are swallowed, no failure in checking the call order would be caught.An "easy" fix would be to change the controller to explicitly catch
APIErrorException
, however this continues the use of "normal exceptions."A better fix would be to refactor
ESTermsQueryService.GetByName
to return null when no term is found and modify the controller accordingly.Steps to reproduce the issue
TermsController.GetByName
, set a breakpoint in thecatch
block of the fallback handling section of the code. (Lines 177-181 in the present code.)TermsControllerTest.GetByName.cs
, debug theGetByName_WithFallback_GeneticsHP
test and allow it to run.What's the expected result?
catch
block should be catchingAPIErrorException
.What's the actual result?
Xunit.Sdk.EqualException
is caught and swallowed without being reported.Additional details / screenshot
Related Tickets