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

Initialize cause of some InterruptedExceptions #1301

Conversation

fedejeanne
Copy link
Contributor

Pack an OperationCanceledException inside the InterruptedException when this is thrown due to the user canceling an operation. This makes it easier to determine the cause of the InterruptedException from outside the methods (i.e. in the catch-clauses). Adapt the method ModalContext::checkCanceled accordingly. Let WorkspaceModifyOperation::run use the original OperationCanceledException if provided and only create a new one as a fallback.

Same idea as #1259

Copy link
Contributor

github-actions bot commented Nov 10, 2023

Test Results

     899 files   -        1       899 suites   - 1   35m 20s ⏱️ - 29m 57s
  7 468 tests ±       0    7 312 ✔️  -        2  153 💤 ±    0  2 +1  1 🔥 +1 
21 977 runs   - 1 576  21 585 ✔️  - 1 458  389 💤  - 120  2 +1  1 🔥 +1 

For more details on these failures and errors, see this check.

Results for commit eae8d35. ± Comparison against base commit 2670545.

♻️ This comment has been updated with latest results.

@fedejeanne
Copy link
Contributor Author

The check failures don't seem to be related to my changes since I haven't changed any API.

@fedejeanne fedejeanne marked this pull request as ready for review November 10, 2023 13:57
@iloveeclipse
Copy link
Member

it's tycho problem, see eclipse-tycho/tycho#3019

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

I agree that the existing exception handling in WorkspaceModifyOperation is not correct, but the fix does seem to solve the problem completey (see my detailed comment).

I do not understand the reason for all the other changes, like I have argued for the according changes to the platform repository here.

@fedejeanne fedejeanne force-pushed the enhancements/init_InterruptedException_upon_user_cancelation branch from 6ec5b2c to b6c33cd Compare November 13, 2023 12:03
@fedejeanne fedejeanne force-pushed the enhancements/init_InterruptedException_upon_user_cancelation branch 2 times, most recently from 12615e9 to 5b150d4 Compare November 14, 2023 13:45
@fedejeanne
Copy link
Contributor Author

I reverted some changes, I hope it's looking good now @HeikoKlare ?

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Change looks good to me now.

@HeikoKlare HeikoKlare force-pushed the enhancements/init_InterruptedException_upon_user_cancelation branch from 1b57e20 to 0f77e30 Compare November 17, 2023 11:58
Pack an OperationCanceledException inside the InterruptedException when
this is thrown due to the user canceling an operation. This makes it
easier to determine the cause of the InterruptedException from outside
the methods (i.e. in the catch-clauses). Adapt the method
ModalContext::checkCanceled accordingly. Change
WorkspaceModifyOperation::run so it preserves the original
exceptions if possible.

Same idea as
eclipse-platform#1259
@fedejeanne
Copy link
Contributor Author

Test failures are unrelated: #1005, #1337 , #1338

@fedejeanne fedejeanne merged commit 0d2fb70 into eclipse-platform:master Nov 29, 2023
13 of 16 checks passed
@fedejeanne fedejeanne deleted the enhancements/init_InterruptedException_upon_user_cancelation branch November 29, 2023 09:02
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.

3 participants