diff --git a/megamek/src/megamek/MegaMek.java b/megamek/src/megamek/MegaMek.java index ae066539dde..ba777df9875 100644 --- a/megamek/src/megamek/MegaMek.java +++ b/megamek/src/megamek/MegaMek.java @@ -92,7 +92,7 @@ public static void main(String... args) { final String name = t.getClass().getName(); final String message = String.format(MMLoggingConstants.UNHANDLED_EXCEPTION, name); final String title = String.format(MMLoggingConstants.UNHANDLED_EXCEPTION_TITLE, name); - logger.error(t, message, title); + logger.errorDialog(t, message, title); }); // Second, let's handle logging diff --git a/megamek/src/megamek/Version.java b/megamek/src/megamek/Version.java index b065b72649f..95ea6483ef6 100644 --- a/megamek/src/megamek/Version.java +++ b/megamek/src/megamek/Version.java @@ -240,7 +240,7 @@ public void fillFromText(final @Nullable String text) { final String nullOrBlank = ((text == null) ? "a null string" : "a blank string"); final String message = String.format(MMLoggingConstants.VERSION_ERROR_CANNOT_PARSE_VERSION_FROM_STRING, nullOrBlank); - logger.fatal(message, MMLoggingConstants.VERSION_PARSE_FAILURE); + logger.fatalDialog(message, MMLoggingConstants.VERSION_PARSE_FAILURE); return; } @@ -249,7 +249,7 @@ public void fillFromText(final @Nullable String text) { if ((snapshotSplit.length > 2) || (versionSplit.length < 3)) { final String message = String.format(MMLoggingConstants.VERSION_ILLEGAL_VERSION_FORMAT, text); - logger.fatal(message, MMLoggingConstants.VERSION_PARSE_FAILURE); + logger.fatalDialog(message, MMLoggingConstants.VERSION_PARSE_FAILURE); return; } @@ -257,7 +257,7 @@ public void fillFromText(final @Nullable String text) { setRelease(Integer.parseInt(versionSplit[0])); } catch (Exception e) { final String message = String.format(MMLoggingConstants.VERSION_FAILED_TO_PARSE_RELEASE, text); - logger.fatal(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); + logger.fatalDialog(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); return; } @@ -265,7 +265,7 @@ public void fillFromText(final @Nullable String text) { setMajor(Integer.parseInt(versionSplit[1])); } catch (Exception e) { final String message = String.format(MMLoggingConstants.VERSION_FAILED_TO_PARSE_MAJOR, text); - logger.fatal(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); + logger.fatalDialog(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); return; } @@ -273,7 +273,7 @@ public void fillFromText(final @Nullable String text) { setMinor(Integer.parseInt(versionSplit[2])); } catch (Exception e) { final String message = String.format(MMLoggingConstants.VERSION_FAILED_TO_PARSE_MINOR, text); - logger.fatal(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); + logger.fatalDialog(e, message, MMLoggingConstants.VERSION_PARSE_FAILURE); return; } diff --git a/megamek/src/megamek/client/ui/swing/ClientGUI.java b/megamek/src/megamek/client/ui/swing/ClientGUI.java index 5f044fdf4b2..5cf420c2f46 100644 --- a/megamek/src/megamek/client/ui/swing/ClientGUI.java +++ b/megamek/src/megamek/client/ui/swing/ClientGUI.java @@ -2250,7 +2250,7 @@ public void printList(ArrayList unitList, JButton button) { // If something goes wrong, probably ProcessBuild.start if anything, // Make sure to set the button text back to what it started as no matter what. button.setText(Messages.getString("ChatLounge.butPrintList")); - logger.error(e, "Operation failed", "Error printing unit list"); + logger.errorDialog(e, "Operation failed", "Error printing unit list"); } } diff --git a/megamek/src/megamek/client/ui/swing/MegaMekGUI.java b/megamek/src/megamek/client/ui/swing/MegaMekGUI.java index 0ec1af35731..1803c71b438 100644 --- a/megamek/src/megamek/client/ui/swing/MegaMekGUI.java +++ b/megamek/src/megamek/client/ui/swing/MegaMekGUI.java @@ -442,9 +442,9 @@ public boolean startServer(@Nullable String serverPassword, int port, boolean is mailProperties.load(propsReader); mailer = new EmailService(mailProperties); } catch (Exception ex) { - logger.error(ex, - Messages.getFormattedString("MegaMek.StartServerError", port, ex.getMessage()), - Messages.getString("MegaMek.LoadGameAlert.title")); + logger.errorDialog(ex, + Messages.getFormattedString("MegaMek.StartServerError", port, ex.getMessage()), + Messages.getString("MegaMek.LoadGameAlert.title")); return false; } } @@ -457,14 +457,14 @@ public boolean startServer(@Nullable String serverPassword, int port, boolean is gameManager = getGameManager(gameType); server = new Server(serverPassword, port, gameManager, isRegister, metaServer, mailer, false); } catch (Exception ex) { - logger.error(ex, - Messages.getFormattedString("MegaMek.StartServerError", port, ex.getMessage()), - Messages.getString("MegaMek.LoadGameAlert.title")); + logger.errorDialog(ex, + Messages.getFormattedString("MegaMek.StartServerError", port, ex.getMessage()), + Messages.getString("MegaMek.LoadGameAlert.title")); return false; } if (saveGameFile != null && !server.loadGame(saveGameFile)) { - logger.error(Messages.getFormattedString("MegaMek.LoadGameAlert.message", saveGameFile.getAbsolutePath()), + logger.errorDialog(Messages.getFormattedString("MegaMek.LoadGameAlert.message", saveGameFile.getAbsolutePath()), Messages.getString("MegaMek.LoadGameAlert.title")); server.die(); server = null; @@ -520,7 +520,7 @@ public void startClient(String playerName, String serverAddress, int port, GameT serverAddress = Server.validateServerAddress(serverAddress); port = Server.validatePort(port); } catch (Exception ex) { - logger.error(ex, + logger.errorDialog(ex, Messages.getFormattedString("MegaMek.ServerConnectionError", serverAddress, port), Messages.getString("MegaMek.LoadGameAlert.title")); return; @@ -617,7 +617,7 @@ public String getDescription() { } } } catch (Exception ex) { - logger.error(ex, + logger.errorDialog(ex, Messages.getFormattedString("MegaMek.LoadGameAlert.message", fc.getSelectedFile().getAbsolutePath()), Messages.getString("MegaMek.LoadGameAlert.title")); @@ -764,15 +764,15 @@ void scenario() { scenario = sl.load(); game = scenario.createGame(); } catch (Exception e) { - logger.error(e, Messages.getString("MegaMek.HostScenarioAlert.message", e.getMessage()), + logger.errorDialog(e, Messages.getString("MegaMek.HostScenarioAlert.message", e.getMessage()), Messages.getString("MegaMek.HostScenarioAlert.title")); return; } // popup options dialog if (!scenario.hasFixedGameOptions() && game instanceof Game twGame) { - GameOptionsDialog god = new GameOptionsDialog(frame, (GameOptions) twGame.getOptions(), false); - god.update((GameOptions) twGame.getOptions()); + GameOptionsDialog god = new GameOptionsDialog(frame, twGame.getOptions(), false); + god.update(twGame.getOptions()); god.setEditable(true); god.setVisible(true); for (IBasicOption opt : god.getOptions()) { diff --git a/megamek/src/megamek/logging/MMLogger.java b/megamek/src/megamek/logging/MMLogger.java index 914c129cb5c..34a95b9bcb3 100644 --- a/megamek/src/megamek/logging/MMLogger.java +++ b/megamek/src/megamek/logging/MMLogger.java @@ -19,8 +19,7 @@ package megamek.logging; -import javax.swing.JOptionPane; - +import io.sentry.Sentry; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -28,27 +27,30 @@ import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.ExtendedLoggerWrapper; -import io.sentry.Sentry; - +import javax.swing.*; import java.util.Collections; /** - * MMLogger - * - * Utility class to handle logging functions as well as reporting exceptions to + *

MMLogger

+ *

Utility class to handle logging functions as well as reporting exceptions to * Sentry. To deal with general recommendations of confirming log level before * logging, additional checks are added to ensure we're only ever logging data - * based upon the currently active log level. - * - * To utilize this class properly, it must be initialized within each class that - * will use it. For example for use with the megamek.MegaMek class, - * - * private static final MMLogger logger = MMLogger.create(MegaMek.class); - * - * And then for use, use logger.info(message) and pass a string to it. Currently - * supported levels include info, warn, debug, error, and fatal. Error and Fatal + * based upon the currently active log level.

+ *

How to use:

+ *

To utilize this class properly, it must be initialized within each class that + * will use it. For example for use with the {@code megamek.MegaMek.class}.

+ * 
{@code private static final MMLogger logger = MMLogger.create(MegaMek.class);}
+ *

And then for use, use {@code logger.info(message)} and pass a string to it. Currently + * supported levels include trace, info, warn, debug, error, and fatal. Warn, Error and Fatal * can take any Throwable for sending to Sentry and has an overload for a title - * to allow for displaying of a dialog box + * to allow for displaying of a dialog box.

+ *

This class also implements both the parametric pattern and the string format + * for the functions that are overriden and added here. This means that you can + * use the following formats for the messages in some cases:

+ * 
{@code logger.info("number: {} string: {}", 42, "Hello");
+ * logger.info("number: %d string: %s", 42, "Hello");}
+ *

Due to how the logger works, the first format using curly braces is preferred, but the second one + * is grandfathered exclusively for logs already existing, and it is not recommended for any new log.

*/ public class MMLogger extends ExtendedLoggerWrapper { private final ExtendedLoggerWrapper exLoggerWrapper; @@ -56,8 +58,7 @@ public class MMLogger extends ExtendedLoggerWrapper { private static final String FQCN = MMLogger.class.getName(); /** - * Private constructor as there should never be an instance of this - * class. + * Package protected constructor, this class should not be created directly. */ MMLogger(final Logger logger) { super((AbstractLogger) logger, logger.getName(), logger.getMessageFactory()); @@ -92,8 +93,7 @@ public static MMLogger create(final Class loggerName) { * Info Level Logging. * * @param message Message to be sent to the log file. - * @param args Variable list of arguments for message to be passed to - * String.format() + * @param args Variable list of arguments for the message */ @Override public void info(String message, Object... args) { @@ -104,10 +104,8 @@ public void info(String message, Object... args) { /** * Warning Level Logging * - * @param message Message to be written to the log file. - * @param args Variable list of arguments for message to be - * passed to String.format() - * + * @param message Message to be logged. + * @param args Variable list of arguments for the message */ @Override public void warn(String message, Object... args) { @@ -118,11 +116,9 @@ public void warn(String message, Object... args) { /** * Warning Level Logging * - * @param exception Exception that was caught via a try/catch block. - * @param message Message to be written to the log file. - * @param args Variable list of arguments for message to be - * passed to String.format() - * + * @param exception Exception to be logged. + * @param message Message to be logged. + * @param args Variable list of arguments for the message */ public void warn(Throwable exception, String message, Object... args) { Sentry.captureException(exception); @@ -133,22 +129,8 @@ public void warn(Throwable exception, String message, Object... args) { /** * Debug Level Logging * - * @param message Message to be written to the log file. - * @param p0 parameter to the message. - * @param p1 parameter to the message. - * @param p2 parameter to the message. - */ - @Override - public void debug(final String message, final Object p0, final Object p1, final Object p2) { - logIfEnabled(FQCN, Level.DEBUG, null, message, p0, p1, p2); - } - - /** - * Debug Level Logging - * - * @param message Message to be written to the log file. - * @param args Variable list of arguments for message to be passed to - * String.format() + * @param message Message to be logged. + * @param args Variable list of arguments for the message */ @Override public void debug(String message, Object... args) { @@ -159,11 +141,9 @@ public void debug(String message, Object... args) { /** * Debug Level Logging * - * @param exception Exception that was caught via a try/catch block. - * @param message Message to be written to the log file. - * @param args Variable list of arguments for message to be - * passed to String.format() - * + * @param exception Exception to be logged. + * @param message Message to be logged. + * @param args Variable list of arguments for the message */ public void debug(Throwable exception, String message, Object... args) { Sentry.captureException(exception); @@ -174,8 +154,8 @@ public void debug(Throwable exception, String message, Object... args) { /** * Error Level Logging w/ Exception * - * @param exception Exception that was caught via a try/catch block. - * @param message Additional message to report to the log file. + * @param exception Exception to be logged. + * @param message Additional message. */ public void error(Throwable exception, String message) { Sentry.captureException(exception); @@ -185,8 +165,9 @@ public void error(Throwable exception, String message) { /** * Error Level Logging w/ Exception * - * @param exception Exception that was caught via a try/catch block. - * @param message Additional message to report to the log file. + * @param exception Exception to be logged. + * @param message Additional message to be logged. + * @param args Variable list of arguments for the message */ public void error(Throwable exception, String message, Object... args) { message = parametrizedStringIfEnabled(Level.ERROR, message, args); @@ -196,11 +177,10 @@ public void error(Throwable exception, String message, Object... args) { /** * Error Level Logging w/ Exception - * * This one was made to make it easier to replace the Log4J Calls * - * @param message Additional message to report to the log file. - * @param exception Exception that was caught via a try/catch block. + * @param message Message to be logged. + * @param exception Exception to be logged. */ public void error(String message, Throwable exception) { Sentry.captureException(exception); @@ -210,7 +190,7 @@ public void error(String message, Throwable exception) { /** * Error Level Logging w/o Exception. * - * @param message Message to be written to the log file. + * @param message Message to be logged. */ @Override public void error(String message) { @@ -219,8 +199,8 @@ public void error(String message) { /** * Error Level Logging w/ Exception w/ Dialog. - * @deprecated (since 01-feb-2025) Use {@link #errorDialog(Throwable, String, String, Object...)} instead. - * @param exception Exception that was caught. + * @deprecated (since 21-feb-2025) Use {@link #errorDialog(Throwable, String, String, Object...)} instead. + * @param exception Exception to be logged. * @param message Message to write to the log file AND be displayed in the * error pane. * @param title Title of the error message box. @@ -232,43 +212,26 @@ public void error(Throwable exception, String message, String title) { /** * Formatted Error Level Logging w/ Exception w/ Dialog. - * + * @param exception Exception to be logged. * @param message Message to write to the log file AND be displayed in the * error pane. * @param title Title of the error message box. + * @param args Variable list of arguments for the message. */ - public void errorDialog(Throwable e, String message, String title, Object... args) { - Sentry.captureException(e); + public void errorDialog(Throwable exception, String message, String title, Object... args) { + Sentry.captureException(exception); message = parametrizedStringAnyway(message, args); - exLoggerWrapper.logIfEnabled(MMLogger.FQCN, Level.ERROR, null, message, e); + exLoggerWrapper.logIfEnabled(MMLogger.FQCN, Level.ERROR, null, message, exception); popupErrorDialog(title, message); } - private String parametrizedStringIfEnabled(Level level, String message, Object... args) { - if (isEnabled(level, null, message) && args.length > 0) { - message = parametrizedStringAnyway(message, args); - } - return message; - } - - - private String parametrizedStringAnyway(String message, Object... args) { - if (args.length > 0) { - if (message.contains("{}")) { - message = new ParameterizedMessage(message, args).getFormattedMessage(); - } else { - message = String.format(message, args); - } - } - return message; - } - /** * Formatted Error Level Logging w/o Exception w/ Dialog. * * @param message Message to write to the log file AND be displayed in the * error pane. * @param title Title of the error message box. + * @param args Variable list of arguments for the message */ public void errorDialog(String title, String message, Object... args) { message = parametrizedStringAnyway(message, args); @@ -282,6 +245,7 @@ public void errorDialog(String title, String message, Object... args) { * @param message Message to write to the log file AND be displayed in the * error pane. * @param title Title of the error message box. + * @param args Variable list of arguments for the message */ private void popupErrorDialog(String title, String message, Object... args) { message = parametrizedStringAnyway(message, args); @@ -322,11 +286,10 @@ public void fatal(Throwable exception, String message) { /** * Fatal Level Logging w/ Exception w/ Dialog * - * @deprecated (since 01-feb-2025) Use {@link #fatalDialog(Throwable, String, String)} instead. + * @deprecated (since 21-feb-2025) Use {@link #fatalDialog(Throwable, String, String)} instead. * @param exception Exception that was triggered. Probably uncaught. * @param message Message to report to the log file. * @param title Title of the error message box. - * */ @Deprecated public void fatal(Throwable exception, String message, String title) { @@ -339,7 +302,6 @@ public void fatal(Throwable exception, String message, String title) { * @param exception Exception that was triggered. Probably uncaught. * @param message Message to report to the log file. * @param title Title of the error message box. - * */ public void fatalDialog(Throwable exception, String message, String title) { fatal(exception, message); @@ -348,10 +310,9 @@ public void fatalDialog(Throwable exception, String message, String title) { /** * Fatal Level Logging w/o Exception w/ Dialog - * @deprecated (since 01-feb-2025) Use {@link #fatalDialog(String, String)} instead. + * @deprecated (since 21-feb-2025) Use {@link #fatalDialog(String, String)} instead. * @param message Message to report to the log file. * @param title Title of the error message box. - * */ @Deprecated public void fatal(String message, String title) { @@ -363,7 +324,6 @@ public void fatal(String message, String title) { * * @param message Message to report to the log file. * @param title Title of the error message box. - * */ public void fatalDialog(String message, String title) { fatal(message); @@ -388,10 +348,28 @@ public boolean isLevelMoreSpecificThan(Level checkedLevel) { * method. * * @param checkedLevel Passed in Level to compare to. - * @return + * @return True if the current log level is less specific than the passed in */ public boolean isLevelLessSpecificThan(Level checkedLevel) { return exLoggerWrapper.getLevel().isLessSpecificThan(checkedLevel); } + private String parametrizedStringIfEnabled(Level level, String message, Object... args) { + if (isEnabled(level, null, message) && args.length > 0) { + message = parametrizedStringAnyway(message, args); + } + return message; + } + + private String parametrizedStringAnyway(String message, Object... args) { + if (args.length > 0) { + if (message.contains("{}")) { + message = new ParameterizedMessage(message, args).getFormattedMessage(); + } else { + message = String.format(message, args); + } + } + return message; + } + } diff --git a/megamek/unittests/megamek/logging/MMLoggerTest.java b/megamek/unittests/megamek/logging/MMLoggerTest.java index ab59761fb09..c0be9df8db6 100644 --- a/megamek/unittests/megamek/logging/MMLoggerTest.java +++ b/megamek/unittests/megamek/logging/MMLoggerTest.java @@ -64,6 +64,13 @@ public void testWarnLogging() { @Test public void testWarnLoggingWithException() { + Exception e = new Exception("Test exception"); + testMMLogger.warn(e, "Warn message: {}", "test"); + verifyLog(Level.WARN, "Warn message: test", e); + } + + @Test + public void testWarnLoggingWithExceptionWithStringFormat() { Exception e = new Exception("Test exception"); testMMLogger.warn(e, "Warn message: %s", "test"); verifyLog(Level.WARN, "Warn message: test", e); @@ -78,6 +85,17 @@ public void testDebugLoggingWithException() { @Test public void testErrorLogging() { + if (GraphicsEnvironment.isHeadless()) { + // Skip this test if running in headless mode + return; + } + automaticallyDismissDialog(); + testMMLogger.errorDialog("test", "Error message: {}", "test"); + verifyLog(Level.ERROR, "Error message: test"); + } + + @Test + public void testErrorLoggingWithStringFormat() { if (GraphicsEnvironment.isHeadless()) { // Skip this test if running in headless mode return; @@ -88,7 +106,7 @@ public void testErrorLogging() { } @Test - public void testErrorLoggingWithExceptionNoParams() { + public void testDeprecatedErrorLoggingWithExceptionNoParams() { if (GraphicsEnvironment.isHeadless()) { // Skip this test if running in headless mode return; @@ -111,7 +129,6 @@ public void testErrorLoggingWithException() { verifyLog(Level.ERROR, "Error message: test", e); } - @Test public void testFatalLogging() { testMMLogger.fatal("Fatal without dialog without exception"); @@ -129,6 +146,17 @@ public void testFatalLoggingWithDialog() { verifyLog(Level.FATAL, "Fatal with dialog with exception"); } + @Test + public void testDeprecatedFatalLoggingWithDialog() { + if (GraphicsEnvironment.isHeadless()) { + // Skip this test if running in headless mode + return; + } + automaticallyDismissDialog(); + testMMLogger.fatal("Fatal with dialog with exception", "Fatal dialog title"); + verifyLog(Level.FATAL, "Fatal with dialog with exception"); + } + @Test public void testFatalLoggingWithException() { Exception e = new Exception("Test exception"); @@ -136,6 +164,18 @@ public void testFatalLoggingWithException() { verifyLog(Level.FATAL, "Fatal without dialog with exception", e); } + @Test + public void testDeprecatedFatalLoggingWithException() { + if (GraphicsEnvironment.isHeadless()) { + // Skip this test if running in headless mode + return; + } + automaticallyDismissDialog(); + Exception e = new Exception("Test exception"); + testMMLogger.fatal(e, "Fatal without dialog with exception" , "Fatal dialog title"); + verifyLog(Level.FATAL, "Fatal without dialog with exception", e); + } + private void verifyLog(Level level, String message) { ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(Message.class); verify(mockLogger).logMessage(anyString(), eq(level), nullable(Marker.class), messageCaptor.capture(), nullable(Throwable.class));