[TESTONLY] Proposal terminate vs precondition#677
Conversation
| { | ||
| score::mw::log::LogFatal("lola") << "MessagePassingClientCache: Cannot close connection to target " | ||
| << target_node_id << " in reasonable time"; | ||
| // no precondition but a runtime error in QNX message passing service --> keep std::terminate() |
There was a problem hiding this comment.
Agree. Even though this could be seen as a postcondition violation. It depends on the functionality of Stop and the expectations of RemoveMessagePassingClient. According to the Stop documentation
Stop seems to do to me, is a stop request that might succeed or fail.Then here we keep getting the state to see if at some point stops, but if not we terminate.
I would leave it as it it with a terminate but improve the API documentation for
RemoveMessagePassingClient and Stop.
| { | ||
| score::mw::log::LogFatal("lola") << "MessagePassingService: Failed to start listening on " << service_identifier | ||
| << " with following error: " << result.error(); | ||
| // no precondition but a runtime error in QNX message passing service --> keep std::terminate() |
There was a problem hiding this comment.
Agree. But imho similar to the other case, it goes direction postcondition violation that we decided that if we do not fulfill the postcondition our reaction is to terminate.
It seems that in the constructor of MessagePassingServiceInstance we want to guarantee that we will be listening but we are failing to do that.
| << "Service discovery lost at least one event and is compromised now. Bailing out!"; | ||
| // Potential optimization: Resync the full service discovery with the file system and update all ongoing | ||
| // searches with potential changes. | ||
| // no precondition, this is a runtime error caused by kernel or filesystem issues / faulty configuration --> keep std::terminate() |
| mw::log::LogFatal("lola") | ||
| << "Directory" << GetSearchPathForIdentifier(enriched_instance_identifier) << "/" << event.GetName() | ||
| << "was deleted. Outside tampering with service discovery. Aborting!"; | ||
| // no precondition, this is a runtime error caused by kernel or filesystem issues / faulty configuration --> keep std::terminate() |
| { | ||
| mw::log::LogFatal("lola") << "Outside tampering! Bailing! Failed to remove flag file" | ||
| << flag_file_path.Native() << ":" << flag_file_result.error(); | ||
| // no precondition but runtime error caused by issues in the filesystem --> keep std::terminate() |
There was a problem hiding this comment.
Hard to say, it depends on the error. There are a lot of reasons why a remove can fail. To me how the check is currently written is too brute force. In the case of "not existing" error, that could be a precondition of the destructor that if is_offered the file must exist. It would not be a precondition based on inputs but based on invariants.
| << "into map. Terminating."; | ||
| std::terminate(); | ||
| } | ||
| SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(insert_result.second, |
There was a problem hiding this comment.
I agree that it is a precondition, but I would phrase the message differently. AFAIK insert can only return false if the element was already in the map. I would phrase the precondition failure in that terms. "shm object handle already in the map, that was not expected"
| << "service element makes no sense. Something has gone wrong! Terminating."; | ||
| std::terminate(); | ||
| } | ||
| SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE( |
| << element_type; | ||
| std::terminate(); | ||
| } | ||
| SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE( |
| << "Invalid call to GetMessagePassingCfg with asil_level B although app/process is configured for QM only."; | ||
| std::terminate(); | ||
| } | ||
| SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE( |
| "doesn't contain a LoLa deployment!"; | ||
| std::terminate(); | ||
| } | ||
| SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE( |
2 participants
Do not merge, PR is only used to discuss future changes.