SAK-47907 Archive UI, logging and error handling improvements

[smarquard]

• Distinguish between success and failure cases and show success or alert
• Handle exceptions from service archive() methods as failure cases
• Include the archive log as part of the archive in archive.xml

[ern]

Have a question about the converting of checked exceptions to a RuntimeException, usually when this is happens it is because an unrecoverable error occurred. I understand that it may mean that a resource may not be exported (like an assignment) and that it is added to the output to the screen and the log as an error (catalina.out as a warning).

Lets make sure we keep clear delineation between the request (itself) failed (ERROR) verses there were errors with exporting the data (WARN).

BTW nice job on adding the log to the archive.

[smarquard]

Having run into cases where some but not all of the assignments were exported, it seems preferable to have the archive() method in the service throw an exception and cause the whole site archive operation to fail rather than complete "silently" (because you wouldn't necessarily notice a warning from one service, or could be archiving via a webservice call) rather than produce an archive which is incomplete.

So it does seem like an ERROR rather than WARN because incomplete data = fail operation rather than incomplete data = give a warning. The types of issues here are where the archiving itself is causing an error of some type like the assignments issue with binary data in a supposedly text field that that won't parse correctly. So there's bad code or code not dealing with data correctly rather than just inconsistent data, and code like this:

https://github.com/sakaiproject/sakai/blob/master/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java#L327

just log a WARN but the user or webservice consumer will never see that detail or know that the archive was incomplete. So a follow-up change to that would be to make the archive() methods in particular services throw exceptions properly rather than swallow them.

I can see that people might think "some archive is better than no archive" though, so a UI setting to "ignore errors" might be a compromise here. But then one might have to pass that all the way down to the archive() methods, e.g. is it better to have no assignments archived, or some assignments archived if at least one can't be archived properly?

[smarquard]

Committed your ERROR>WARN suggestions above, @ern. Any further thoughts on this?

[ern]

Thanks for the work Stephen