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

Do not log thrown Mistake #1561

Merged
merged 16 commits into from
Dec 15, 2024
Merged

Do not log thrown Mistake #1561

merged 16 commits into from
Dec 15, 2024

Conversation

alexander-yevsyukov
Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov commented Dec 14, 2024

This PR updates the behaviour of Bus.doPost() to avoid logging instances of Mistake thrown during dispatching.
Also, the code of AbstractReceptor was updated to treat catching Mistake in a special way, instead of Error, as was done previously.

@alexander-yevsyukov alexander-yevsyukov self-assigned this Dec 14, 2024
@alexander-yevsyukov alexander-yevsyukov marked this pull request as ready for review December 14, 2024 16:17
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.77%. Comparing base (2692136) to head (b0c67ef).
Report is 17 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1561      +/-   ##
============================================
- Coverage     89.79%   89.77%   -0.03%     
+ Complexity     5023     5022       -1     
============================================
  Files           646      646              
  Lines         15759    15761       +2     
  Branches        921      921              
============================================
- Hits          14151    14149       -2     
- Misses         1276     1280       +4     
  Partials        332      332              

@alexander-yevsyukov alexander-yevsyukov marked this pull request as draft December 14, 2024 21:22
@alexander-yevsyukov alexander-yevsyukov changed the title Do not log thrown Error Do not log thrown Mistake Dec 14, 2024
@alexander-yevsyukov alexander-yevsyukov marked this pull request as ready for review December 14, 2024 22:33
@alexander-yevsyukov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-yevsyukov LGTM except for a comment.

* <p>If {@link #dispatch(SignalEnvelope) dispatching} throws a {@link Mistake} it is rethrown.
* Other types of exceptions are logged and then rethrown.
*
* <p>We treat {@link Error}s differently and want to void much of console output
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We treat Mistakes differently. Is that what you meant?

@armiol
Copy link
Contributor

armiol commented Dec 15, 2024

@alexander-yevsyukov also, the license report check failed. JFYI.

@armiol
Copy link
Contributor

armiol commented Dec 15, 2024

@alexander-yevsyukov

     * Please see {@link io.spine.server.model.AbstractReceptor#invoke(Object, MessageEnvelope)}
     * for more details on special treatment of {@link Error} during dispatching.

There's still about Errors. Not about Mistakes.

server/src/main/java/io/spine/server/bus/Bus.java Outdated Show resolved Hide resolved
* <p>We treat {@link Mistake}s differently and want to void much of console output
* for this special case of exceptions.
* Please see {@link io.spine.server.model.AbstractReceptor#invoke(Object, MessageEnvelope)}
* for more details on special treatment of {@link Message} during dispatching.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be

for more details on special treatment of {@link Mistake} during dispatching.

@armiol
Copy link
Contributor

armiol commented Dec 15, 2024

@alexander-yevsyukov please see my last comment. It's not addressed yet.

@alexander-yevsyukov alexander-yevsyukov merged commit d9fa60a into master Dec 15, 2024
7 checks passed
@alexander-yevsyukov alexander-yevsyukov deleted the do-not-log-error branch December 15, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants