sofa icon indicating copy to clipboard operation
sofa copied to clipboard

[runSofa] improve exception handling

Open damienmarchal opened this issue 3 years ago • 6 comments

Currently in Sofa we aren't catching any exceptions.

The consequence is that every exception propagates to the top level exception handler halting the application. Un-handled exception are useful to detect bugs but this is a non-sense to use such top level handler to process possible application errors like failure to load a plugin or a scene.

A good approach should be that any exception that can be fixed without editting the source code (ie. but by editting the scene or changing some system configuration and so on) should ends with a gracefull error message indicating to the user how to fix the error cause.

To implement this I suggest to:

  • add a close to top-level try/catch block around simulations/plugin actions
  • add a SofaSimulationException that has an error message, the component that emitted it and can have a backtrace as well as a top level reactions to do (eg: set componentstate to invalid and stop simulation).

We should also use this SofaSimulationException at any place we are using msg_error(). (Actually it could even make sense to replace the msg_error macro by throwing the exception).


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

damienmarchal avatar Sep 15 '21 09:09 damienmarchal

[ci-build][with-all-tests]

fredroy avatar Sep 20 '21 14:09 fredroy

Thanks for the review @alxbilger,

I integrates your comments. More generally do you have any opinion on how we could improve the exceptions/component error in Sofa ?

damienmarchal avatar Sep 27 '21 10:09 damienmarchal

Yes @damienmarchal

My opinion is that introducing exception can not be done in one PR without taking the time to design a global API. If we do not only rely on the stl exceptions, we need to define the specs of the desired exception API: for documentation purposes, and most importantly to avoid having two error management systems in parallel and the exception not being well propagated and used.

Let 's discuss this tomorrow ;)

hugtalbot avatar Sep 28 '21 14:09 hugtalbot

Poke @hugtalbot I re-open it because there is the need for that :)

damienmarchal avatar Mar 23 '22 10:03 damienmarchal

:+1: this is reopened related to #2287

hugtalbot avatar Mar 23 '22 10:03 hugtalbot