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

Handle multiple errors in same ErrorDialog #2933

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

HebaruSan
Copy link
Member

Problem

If you start downloading multiple modules in GUI and more than one of them fail, you'll get an unhandled exception like this:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: The form is already displayed as a modal dialog.
  at System.Windows.Forms.Form.ShowDialog (System.Windows.Forms.IWin32Window owner) [0x00081] in <f73fa304d089422f9e23b25394111843>:0 
  at System.Windows.Forms.Form.ShowDialog () [0x00000] in <f73fa304d089422f9e23b25394111843>:0 
  at CKAN.ErrorDialog.ShowErrorDialog (System.String text, System.Object[] args) [0x00071] in <ea98fd2118b84fc7ad1a2141a259ca95>:0 

Cause

In #2756 we started handling individual module download success or failure immediately, regardless of whether other downloads were still in progress. This opened the possibility of multiple calls to ErrorDialog.ShowErrorDialog without the error dialog being closed in between, and Form.ShowDialog throws the above exception when the form is already visible.

Changes

  • Now ErrorDialog.ShowErrorDialog only calls ShowDialog if Visible is false, to avoid throwing the exception
  • Now if ErrorDialog is already visible, additional messages will be appended to the main one
    • The text is cleared when the form is closed
    • The default text is now empty, so the first message won't be appended to "Error!" or "Fehler!"
  • Now ErrorDialog is resizable and has anchored controls to fit the space
  • Now ErrorDialog is wider by default
  • Now ErrorDialog.ShowErrorDialog resizes the form to fit its content, up to a maximum height of 600 pixels, to make it easier to read the messages, just like Auto size yes no dialog #2729
  • Now ErrorDialog will open centered on the main window instead of centered on the screen
  • ErrorDialog no longer inherits from FormCompatibility because it doesn't help and was messing with the size

Fixes #2890.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Pull request Network Issues affecting internet connections of CKAN labels Nov 25, 2019
@HebaruSan HebaruSan requested a review from DasSkelett November 25, 2019 19:04
Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Works very well, now you can see the full beauty of multiple stack traces!

@HebaruSan HebaruSan merged commit 0bfebd4 into KSP-CKAN:master Nov 25, 2019
@HebaruSan HebaruSan deleted the fix/multi-err-dialog branch November 26, 2019 18:28
@DasSkelett DasSkelett added the i18n Issues regarding the internationalization of CKAN and PRs that need translating label Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] CKAN crashes when multiple mods fail to download
2 participants