Flaws in the Patch-Export and Startup Error-Handling: unhandled cases and unfinished logs

[ogerboss]

When you encounter an exception at the export stage, SkyProc will show you a pop-up and then quit. So far, so good, but there are two flaws:

• the last fraction of the logs is missing, seems like their buffers are not flushed at the end: (from SUMGUI, line 961)

                            JOptionPane.showMessageDialog(null, "There was an error exporting the custom patch.\n(" + ex.getMessage() + ")\n\n" + errorMessage);
                            SPGlobal.closeDebug(); //adding this explicit call here did the trick for me
                            exitProgram(false, true);

• some error types are not handled internally and just give you a "(null)" as reason. Together with the cut-off logs, this leaves the user that encounters this in a very confused state.

The stacktrace from the export error, I got with the above hack was:

#!

[37]                                 [EXCEPTION]  java.lang.NullPointerException
    at skyproc.WEAP$DNAM.export(WEAP.java:102)
    at skyproc.SubRecords.export(SubRecords.java:45)
    at skyproc.MajorRecord.export(MajorRecord.java:244)
    at skyproc.GRUP.export(GRUP.java:155)
    at skyproc.Mod.export(Mod.java:756)
    at skyproc.Mod.export(Mod.java:661)
    at skyproc.Mod.export(Mod.java:638)
    at skyproc.gui.SUMGUI$ProcessingThread.run(SUMGUI.java:956)
    at java.lang.Thread.run(Unknown Source)

In this particular case, Tes5Edit told me that the DNAM data block was completely missing (empty fields) in the responsible mod. I think it would be a good idea to write a warning about corrupt data in such cases and provide the user with the responsible formID.

UPDATE: The startup phase (There has been an error... please refer to the debug logs.) also suffers from the missing buffer flush issue.

[David Tynan]

I'll add SPGlobal.closeDebug(), the logs should always be flushed and closed before before you exit, dunno why it wasn't there. Do you know what error types give null?

It would be nice to have sanity checks but it would be a lot of work at this point. Its a possible idea for the future though. (Or if someone else wants to take up the torch I would sketch out how I would do it and advise as needed.)

[ogerboss]

I am only aware of the issue with the missing DNAM-block I already described in my initial report.

I suggest that you simply add a default case like "unexpected problem, please check the debug log for details" and the FormID of the troublesome record. Together with the final buffer flush it should suffice to pinpoint most issues.

[ogerboss]

added new insights to description: the startup phase also has the missing BufferFlush issue.

[ogerboss]

As far as I see it, merging pull request #3 should resolve this issue, because both exceptions in OnStart and the Export stage are then handled by the ExceptionManager. So you would at least get a proper stacktrace for those (null) cases in the debug overview.