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

Refactor: investigate cleaner exception handling for server/server.cpp #7787

Closed
mofosyne opened this issue Jun 6, 2024 · 3 comments · Fixed by #9056
Closed

Refactor: investigate cleaner exception handling for server/server.cpp #7787

mofosyne opened this issue Jun 6, 2024 · 3 comments · Fixed by #9056
Labels
help wanted Extra attention is needed refactoring Refactoring

Comments

@mofosyne
Copy link
Collaborator

mofosyne commented Jun 6, 2024

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 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.
@mofosyne mofosyne added the refactoring Refactoring label Jun 6, 2024
@mofosyne mofosyne changed the title Refactor: investigate cleaner exception handling Refactor: investigate cleaner exception handling for server/server.cpp Jun 6, 2024
@HanClinto
Copy link
Collaborator

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).

@0wwafa
Copy link

0wwafa commented Jun 6, 2024

Examples:
image

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.

@skoulik
Copy link

skoulik commented Jun 6, 2024

Also related: #7073, #7589

@mofosyne mofosyne added the help wanted Extra attention is needed label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed refactoring Refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants