07th-mod / python-patcher

Mod Installer for the Higurashi and Umineko Games
151 stars 12 forks source link

Port nice looking error/exception popup from "loading" page to "installer" page #143

Closed drojf closed 3 years ago

drojf commented 3 years ago

Currently, if the loading page has an error, it displays a nice popup like this:

unknown

It would be good if this were ported to the installer page, as it is not only nicer for users, but also would increase the rate that people report bugs to us.

drojf commented 3 years ago

On a related note, if the installData.json version makes your installer out-of-date, you get this error message:

installer_init_installData_version

The popup you get on the index.html page is much nicer looking, and has button you can click to get a new version of the installer. Ideally we should show the same prompt and/or show the error message in a nicer way.

drojf commented 3 years ago

Bumping this to remind myself, that this is actually important

Recently we had an issue where a user got a 'permission denied' error, even though our installer now runs as administrator by default (see https://github.com/07th-mod/python-patcher/issues/114)

When you get this error, the installer will display an error Message with a link to this FAQ:

--------------- EXECUTION FINISHED ---------------

Install Failed!: 
SevenZip Extraction Failed - See Details

----- Details -----
SevenZipException: Permission error: See our installer wiki FAQ about this error at https://07th-mod.com/wiki/Installer/faq/#extraction-stage-fails-i-get-an-acess-denied-error-when-overwriting-files

 Could not extract [Umineko Answer (Ch. 5-8) Downloads\umineko-qa-bgm-restoration.zip]
Exception in thread Thread-103:
Traceback (most recent call last):
  File "D:\obj\windows-release\37win32_Release\msi_python\zip_win32\threading.py", line 926, in _bootstrap_inner
  File "D:\obj\windows-release\37win32_Release\msi_python\zip_win32\threading.py", line 870, in run
  File "C:\Users\rouge\AppData\Local\Temp\.tmpVDF61J\httpGUI.py", line 665, in errorPrintingInstaller
    installerFunction(args)
  File "C:\Users\rouge\AppData\Local\Temp\.tmpVDF61J\uminekoInstaller.py", line 163, in mainUmineko
    downloaderAndExtractor.extract(remapPaths)
  File "C:\Users\rouge\AppData\Local\Temp\.tmpVDF61J\common.py", line 1007, in extract
    destinationFileName)
  File "C:\Users\rouge\AppData\Local\Temp\.tmpVDF61J\common.py", line 722, in extractOrCopyFile
    raise SevenZipException("{}\n\n Could not extract [{}]".format(monitor.getErrorMessage(), sourcePath))
common.SevenZipException: Permission error: See our installer wiki FAQ about this error at https://07th-mod.com/wiki/Installer/faq/#extraction-stage-fails-i-get-an-acess-denied-error-when-overwriting-files

 Could not extract [Umineko Answer (Ch. 5-8) Downloads\umineko-qa-bgm-restoration.zip]

One of the items in the FAQ is to "check your antivirus software" however the user didn't visit the FAQ - and it turned out that Norton Antivirus was blocking the installer from overwriting files.

The "detailed exception information" definitely doesn't help. I think I put it in there so that I could read the stack trace from users who copy the error message, but I don't think it's worth it, as (even I) tend to give up pretty quickly if I see a large error message.

So in summary:

  1. Use nice looking error message (perhaps fashion more as "what to try next")
  2. Remove stack trace from error message
  3. Test this permission denied error message and other error messages to make sure they are displayed properly.
  4. For this permission denied error message in particular, provide some brief instructions in the installer, then refer to FAQ.
drojf commented 3 years ago

Mostly fixed in c69a4de549103465b06cdcf721aee096360af691 dd4de9e8c33d0e73094bf8c9bd510d1e76243212, released in https://github.com/07th-mod/python-patcher/releases/tag/v1.1.85

For items 3 and 4 in the list, my idea was to allow markdown in the error messages, but I think that should be its own issue. I'm not sure many errors would benefit from having that (and I would have to then go through all the errors and check if they need rewriting).

I also stated in the above comment to put some information directly into the error message rather than having to go to our FAQ, but having a link to the FAQ allows me to update the information without doing a new installer release.

So for the above reasons I'm closing the issue, even though 3 and 4 are not completed.