[ISSUE #3830] InterruptedExceptions should not be ignored in the code. #4791
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!
Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!
Want to get closer to the community?
| WeChat Assistant | WeChat Public Account | Slack |
|---|---|---|
 |
 |
Join Slack Chat |
Mailing Lists:
| Name | Description | Subscribe | Unsubscribe | Archive |
|---|---|---|---|---|
| Users | User support and questions mailing list | Subscribe | Unsubscribe | Mail Archives |
| Development | Development related discussions | Subscribe | Unsubscribe | Mail Archives |
| Commits | All commits to repositories | Subscribe | Unsubscribe | Mail Archives |
| Issues | Issues or PRs comments and reviews | Subscribe | Unsubscribe | Mail Archives |
There was a problem hiding this comment.
@jevinjiang Thanks for you contribution. Please fix ci, the code style
|
Sorry, I didn't notice before, but now it has passed the checkStyle @mxsm |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4791 +/- ##
=============================================
+ Coverage 0 16.33% +16.33%
- Complexity 0 1735 +1735
=============================================
Files 0 856 +856
Lines 0 31269 +31269
Branches 0 2699 +2699
=============================================
+ Hits 0 5108 +5108
- Misses 0 25683 +25683
- Partials 0 478 +478 ☔ View full report in Codecov by Sentry. |
| } catch (InterruptedException e) { | ||
| log.error("Send message InterruptedException", e); | ||
| Thread.currentThread().interrupt(); // Restore interrupted status | ||
| throw this.checkProducerException(msg.getTopic(), messageId, e); |
There was a problem hiding this comment.
Since you have chosen to re-interrupt, is there no need to throw an exception again? Moreover, the throws are all non-InterruptedExceptions.
既然您选择了重新中断是不是就没必要再抛异常了？更何况抛出都是一些非InterruptedException。
There was a problem hiding this comment.
If I don't throw an exception, I will return a SendResult, so I chose to throw a StorageRuntimeException. Is there any better way? I can continue to modify it. 不抛出异常的话,就要返回一个SendResult,所以我选择了抛出一个StorageRuntimeException, 还有什么更好的做法没,我可以继续修改
There was a problem hiding this comment.
I think you can either return an empty SendResult, or you can use the other way of handling InterruptedException mentioned in the issue, "InterruptedExceptions should either be rethrown - immediately...".
我觉得您可以返回空的SendResult，或者采用issue中提到的另一中处理InterruptedException的方式“直接抛出中断异常”。
There was a problem hiding this comment.
Okay, it's done. Thank you for your guidance in helping me complete the first issue。 已经修改完毕，感谢您帮我完成第一个issue。
| @@ -68,7 +68,7 @@ public void setExtFields() { | |||
| super.getRocketmqProducer().setCompressMsgBodyOverHowmuch(10); | |||
| } | |||
|
|
|||
| public SendResult send(CloudEvent cloudEvent) { | |||
| public SendResult send(CloudEvent cloudEvent) throws InterruptedException { | |||
There was a problem hiding this comment.
Your modification is not much different from the original code, because the catch here does not throw InterruptedException, so it still swallows InterruptedException. What I mean is to throw InterruptedException directly in catch(InterruptedException) or return empty SendResult in catch(InterruptedException).
您这样修改和原代码区别不大，因为这里的catch不会抛出InterruptedException，所以还是将InterruptedException吞掉了。我的意思是在catch(InterruptedException)中直接抛出InterruptedException或者返回空的SendResult。
There was a problem hiding this comment.
I'm very sorry again. QAQ
…e code. (apache#4791) * [ISSUE apache#3830] InterruptedExceptions should not be ignored in the code. * [ISSUE apache#3830] fix code style * [ISSUE apache#3830] delete Unneeded throw * [ISSUE apache#3830] rethrown InterruptedException * [ISSUE apache#3830] InterruptedException return new SendResult --------- Co-authored-by: JiangShuJu <[email protected]>
Fixes #3830
Motivation
Add catch InterruptedException and ThreadDeath exceptions in the code.
Modifications
Documentation