Crash on update with cbjavaloader
cbjavaloader is installed by default with advanced script template (default for coldbox create app). Update crashes when the server is running. try/catch maybe?
I agree that the error message is descriptive enough. If that message shows up in a usable form in a catch block, it would be nice to say “update failed” (in red text?) and dump the message so I know why. Whether you abort the update operation completely or just the component that failed, continuing on with the rest of the updates, is up to you. This really should only happen with a jar file and if I was notified that it failed I could make the decision to restart the service and try again if it was that important to me.
At the end of the day, it’s your baby. I wouldn’t presume to dictate behavior. I just feel that, from a user perspective, seeing a stack trace isn’t as elegant as it could be. It gives me the sense that the operation is half completed and therefore not reliable. Was the part that crashed the only thing affected? It doesn’t live up to the high level of quality you’re known for.
Whether the update should continue depends on your perspective I suppose. If the entire update can’t complete, then the app will still likely be unusable and and packages that can update will need to be addressed before the user can move on. I don’t really see a scenario where 3 packages install successfully, 1 fails, and the user just shrugs and keeps testing as though the failure wasn’t a blocker.
As far as a graceful error message, this is typically harder than it sounds. Sure in this case we are able to guess at what the actual problem was, but if we put in a message that makes assumptions about why the install failed we end up with an error message that is misleading if the next install fails due to
files locked by another process
unrelated application errors that just happen to be inside the same try/catch
I’ve already run into this exact issue with ForgeBox’s error handling in CommandBox where I tried to put in “Helpful” error messages only to have them be confusingly incorrect when a similar error happened but for a different reason I’m not even sure if the actual Java exception class is consistent here to detect an exception on any file system as a result of file system locks vs other exceptions in the underlying file system.
If you want, you can do some testing to try and figure out what java exception classes are usually thrown for this sort of error and we can try and find a message that explains that, but to be honest, the message that is already thrown by the JVM is pretty on point already! I mean, it tells you exactly what I would say-- It gives you the exact file path and then says it can’t be accessed because it’s locked by another process. Any other context we try and work into a message is really just us guessing as to the “why”. That message is basically exactly what I would recommend telling the user.
While it should terminate updating cbjavaloader, leaving that one module outdated, it shouldn’t crash out as it is. Returning a message that says something like “cbjavaloader (or some java jar file it relies on) is currently in use - You’ll need to stop the service to update this” and continuing with the rest of the updates would be most elegant. But it feels to me like even a full abort, done gracefully, would be better than crashing with a stack trace.
As annoying as this may be, it’s more/less working as designed. When you install a package, it has to be uninstalled first. And when CF is running, it locks any jars it has loaded. As such, updating a package that contains jars currently in use by the JVM isn’t going to work. I could try/catch it, but the error message that’s thrown is already about as descriptive as I’d want. And it makes sense that the failed uninstallation terminates the current update command.