iox-#654 stop flooding output if there is an outdated socket

[elBoberido]

Pre-Review Checklist for the PR Author

1. Code follows the coding style of CONTRIBUTING.md
2. Tests follow the best practice for testing
3. Branch follows the naming format (iox-#123-this-is-a-branch)
4. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#123 commit text)
   • Commit messages are signed (git commit -s)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
5. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
6. Relevant issues are linked
7. Add sensible notes for the reviewer
8. All checks have passed (except task-list-completed)
9. Assign PR to reviewer

Notes for Reviewer

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• Unit tests have been written for new behavior
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Closes Starting an application after RouDi was terminated floods the cmd line with error messages #654

[elBoberido]

@elfenpiff I have the feeling we need something like evaluateWithSilentErrnos. What do you think?

[mossmaurice]

I'm unsure if I like the idea. Then the decision is not binary anymore and we have three categories: errors, silent errors and ignored errors. In this case it would just mean the check for ECONNREFUSED is moved from happy path to the sad path, right?

[elfenpiff]

@elBoberido I think I am slightly against it. But I would support that we get rid of evalluateWithIgnoredErrnos and replace it with evaluateWithSilentErrnos I feel that it would make much more sense. Then we could get rid of the errnum in the success case since here it would be always errno = 0.
Yeah I like it!

[elBoberido]

@mossmaurice it is already the sad path but upgraded to the happy path to prevent logging to the console and then doing the same error handling like in the sad path. I've seen it also in one or two other places

@elfenpiff I think it's valuable to to completely ignore some errnos and handle them also as success but at the same time it is also necessary to not ignore the errno because it it still the error path but to not spam the output because it's an expected error which can easily be recovered from. What do you think of the following

class PosixCallEvaluator
{
  public:
    PosixCallEvaluator<ReturnType> successReturnValue(...) && noexcept;
    PosixCallEvaluator<ReturnType> failureReturnValue(...) && noexcept;
    
    PosixCallEvaluator<ReturnType> ignoredErrno(...) && noexcept;
    PosixCallEvaluator<ReturnType> silencedErrno(...) && noexcept;

    cxx::expected<PosixCallResult<ReturnType>, PosixCallResult<ReturnType>> evaluate() const&& noexcept;

  private:
    bool m_isError;
    bool m_isSilent;
    int m_returnValue;
    int m_errnum;
};

With 0 as default value for success return this would look like this

posixCall(foo)().evaluate();
posixCall(bar).failureReturnValue(5).evaluate();
posixCall(baz).silencedErrnos(ENOENT, ECONNREFUSED).evaluate();

This would also make it easier to extend the evaluation in the future if we encounter a weird behavior. What do you think?

[codecov]

Codecov Report

Merging #804 (0f14478) into master (d3f697f) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master     #804      +/-   ##
==========================================
- Coverage   74.36%   74.35%   -0.02%     
==========================================
  Files         322      322              
  Lines       11533    11532       -1     
  Branches     1956     1956              
==========================================
- Hits         8577     8575       -2     
- Misses       2193     2194       +1     
  Partials      763      763              

Flag	Coverage Δ
unittests	73.17% <50.00%> (-0.05%)	⬇️
unittests_timing	30.77% <50.00%> (-0.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
..._hoofs/source/posix_wrapper/unix_domain_socket.cpp	59.05% <50.00%> (-2.33%)	⬇️
iceoryx_hoofs/source/posix_wrapper/timer.cpp	62.33% <0.00%> (+1.73%)	⬆️

[mossmaurice]

LGTM, gave it a try and there was no output anymore \o/

[mossmaurice]

I'm unsure if I like the idea. Then the decision is not binary anymore and we have three categories: errors, silent errors and ignored errors. In this case it would just mean the check for ECONNREFUSED is moved from happy path to the sad path, right?

[elfenpiff]

@elBoberido I think I am slightly against it. But I would support that we get rid of evalluateWithIgnoredErrnos and replace it with evaluateWithSilentErrnos I feel that it would make much more sense. Then we could get rid of the errnum in the success case since here it would be always errno = 0.
Yeah I like it!