You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In #7642@0wwafa observed unhanded exceptions on windows, but didn't provide a concrete example of a failure to load replications steps, so the root cause is unknown. He assert that every exception should be catched globally.
However @ngxson mentioned that we should do a more local try catch focused on ctx_server.init() as it makes no sense to cover ctx_server.load_model(params) which will always return false. 0wwafa opted afterwards to close the PR.
Considering that he did observe an exception around this area, we should at least give this spot a lookover to ensure all error cases are handled.
If done so, make sure to at least credit @0wwafa in the commit for the general observation.
Possible Refactor Approaches
See if we can restrict the error handling to ctx_server.init() or closer to the error source.
The text was updated successfully, but these errors were encountered:
mofosyne
changed the title
Refactor: investigate cleaner exception handling
Refactor: investigate cleaner exception handling for server/server.cpp
Jun 6, 2024
Possibly related to #7391 as an extension / application of this -- in that case, the exception / error is being swallowed (not thrown), but it's still related to our overall approach of dealing with errors when in server mode (whether streaming or not).
the simplest example is to run server.exe with a non existing model or wrong parameters.. the load fails (obviously) the exception is not caught and the program crashes.
Background Description
In #7642 @0wwafa observed unhanded exceptions on windows, but didn't provide a concrete example of a failure to load replications steps, so the root cause is unknown. He assert that every exception should be catched globally.
However @ngxson mentioned that we should do a more local try catch focused on
ctx_server.init()
as it makes no sense to coverctx_server.load_model(params)
which will always return false. 0wwafa opted afterwards to close the PR.Considering that he did observe an exception around this area, we should at least give this spot a lookover to ensure all error cases are handled.
If done so, make sure to at least credit @0wwafa in the commit for the general observation.
Possible Refactor Approaches
The text was updated successfully, but these errors were encountered: