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

ErrorHandling concept with user defined actions #1032

Open
2 tasks
elfenpiff opened this issue Jan 21, 2022 · 5 comments · Fixed by #1902, #2146, #2147, #2181 or #2198
Open
2 tasks

ErrorHandling concept with user defined actions #1032

elfenpiff opened this issue Jan 21, 2022 · 5 comments · Fixed by #1902, #2146, #2147, #2181 or #2198
Assignees
Labels
enhancement New feature feature request Feature request under evaluation globex
Milestone

Comments

@elfenpiff
Copy link
Contributor

elfenpiff commented Jan 21, 2022

Brief feature description

In a safety certified environment the user may would like to define what reaction certain errors cause. Like:

  • call to std::terminate
  • throw an exception
  • inform some watch dog and go into a spinlock

Furthermore, to avoid death tests in unit tests customizing the behavior so that exceptions can be thrown can be useful. This would require to get rid of noexcept. The idea would be that iceoryx never throws an exceptions but allows the user to overload/define the error handling so that it is capable of throwing custom exception which is at the moment impossible since noexcept is in place.

Details

Iceoryx as a framework should enable the user to pursue any error concept they like with a fitting interface. This error concept should then be used uniformly in all iceoryx components so that we have one single call like errorHandler( instead of a wild mixture of std::terminate, cxx::Expects, cxx::Ensures, assert etc.

This call may require the origin, cause, severity of the error so that user defined callbacks can be defined to react differently based on origin, severity and cause.

@elBoberido fell asleep on his keyboard and maybe the products of his dreams can provide us with further insights, see here

Definition of done

The error handling concept is documented in: doc/design/error-handling.md. For this the current error handling concept may have to be discarded/rewritten.

Follow-up issues can then implement the error concept.

Further insights

We have the ability to allow the user to throw exceptions in callbacks, cxx::function or cxx::function_ref but noexcept will cause always a std::terminate.
This is especially a problem in tests where we avoid fatal errors with Expects or the error handler. This is untestable since the errorhandler calls the custom handler and then continues - we require an exception here to make the code testable.

Think of testing this piece of code:

void doStuff(someClass * ptr) {
  iox::cxx::Expects(ptr != nullptr);

  ptr->doStuff();
}

A test like

auto handle =
    iox::ErrorHandler::setTemporaryErrorHandler([&](auto, auto, auto) { wasErrorHandlerCalled = true; });
doStuff(nullptr);
EXPECT_TRUE(wasErrorHandlerCalled);

will always end up with a segfault since the code in doStuff continues after the error handler call. But if we would allow exceptions we could throw an exception in the error handler and safely return to the test.
Then the test only has to verify that the exception was thrown and this indicates that we correctly used the error handler in doStuff.

Requirements

Open for discussions

  • exchange error handling on compile time, switch on runtime is forbidden
    • must be defined via iceoryx_platform and stored in platform
  • no dependencies to iceoryx_hoofs
  • it should support exceptions, the whole iceoryx code base should support exceptions (remove noexcept)
  • 3 error levels, moderate, error and fatal
    • on fatal the error handler must be blocking

Todo

@elfenpiff elfenpiff added enhancement New feature feature request Feature request under evaluation labels Jan 21, 2022
@mossmaurice
Copy link
Contributor

@elfenpiff Thanks for creating this error! I checked the SEI CERT C++ rule-set mentioned in the CONTRIBUTING.md: It's quite plausible that terminating the program abruptly without calling the d'tor may cause security issues e.g. left-over open sockets. See rule ERR50-CPP.

@MatthiasKillat
Copy link
Contributor

MatthiasKillat commented Jun 13, 2022

Working on this now. Starting with a list of requirements followed by a prototype. Will be based on similar principles as the logger to allow defining it on a platform basis.

MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 14, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 16, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 17, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 17, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 17, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 17, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 17, 2022
MatthiasKillat added a commit to ApexAI/iceoryx that referenced this issue Jun 21, 2022
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 4, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 4, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 4, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 4, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 4, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit to elBoberido/iceoryx that referenced this issue Feb 5, 2024
elBoberido added a commit that referenced this issue Feb 6, 2024
…-with-iox-enforce

iox-#1032 replace 'IOX_EXPECTS'/'IOX_ENSURES' with 'IOX_ENFORCE'
@elBoberido elBoberido linked a pull request Feb 6, 2024 that will close this issue
21 tasks
elBoberido added a commit that referenced this issue Feb 19, 2024
iox-#1032 Add stringified condition to the error output
@elBoberido elBoberido linked a pull request Feb 19, 2024 that will close this issue
22 tasks
@github-project-automation github-project-automation bot moved this to In progress in v3.0 Aug 29, 2024
@mossmaurice
Copy link
Contributor

@MatthiasKillat Is there anything still to be done for the release v3.0 in this issue?

@elBoberido
Copy link
Member

@mossmaurice the issue description does not fit anymore and should re rewritten to what was actually done.

@mossmaurice
Copy link
Contributor

mossmaurice commented Jan 6, 2025

@elBoberido Let's sync on this in the next dev meetup and clean up this issue together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment