Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging: Default Global Exception Handler and Standardized Legacy Logging #993

Conversation

Windchild292
Copy link
Contributor

MegaMekLab side of MegaMek/megamek#3346

@Windchild292 Windchild292 added the (RFE) Enhancement Requests for Enhancement, new features or implementations label Jan 2, 2022
@Windchild292 Windchild292 self-assigned this Jan 2, 2022
@SJuliez
Copy link
Member

SJuliez commented Jan 2, 2022

Interesting. Does this affect the AWT event thread? I'd expect exceptions to mostly happen there.

@Windchild292
Copy link
Contributor Author

Windchild292 commented Jan 2, 2022

I tested using AWT, for exactly that reason. Plus it's a thread we don't directly control, which ensures that it holds across the system.

With AWT Exceptions it won't always do the graphical display, but does properly log the exception to the primary log file (MegaMekLab.log for MML).

@Windchild292
Copy link
Contributor Author

image

@Windchild292
Copy link
Contributor Author

I will be watching the legacy log though, to ensure everything is caught. Once I'm done converting the remaining legacy logging to the log manager I'll remove the clearing functionality, and hopefully we will see an empty log from our constant dev version players. If those stay empty for a few releases, I'll know we can (finally) remove the old redirect.

@SJuliez
Copy link
Member

SJuliez commented Jan 2, 2022

Just for the record, I gave it a quick test by changing line 48 in MegaMekLabMainUI to
menubarcreator = new MenuBar(null);
(replace "this" with "null"). It just closed down MML without any dialog popping up. But the exception is still in the log file, so all in all I experienced no change.

@Windchild292
Copy link
Contributor Author

Windchild292 commented Jan 2, 2022

It does log on the proper log file though, not the legacy one... right? If so, then it's still an improvement for that case.

Otherwise... I'll take a second look because that's not how it's working on my end

@SJuliez
Copy link
Member

SJuliez commented Jan 2, 2022

It appeared in megameklab.log. Is megameklablog.txt the legacy one then? (Sorry Im a bit oblivious there, I tend to comment out the logging to have exceptions in the console)

@Windchild292
Copy link
Contributor Author

Legacy.log will be the new legacy logger, which is the console redirect. LogManager writes to MegaMekLab.log, so it's an improvement above the current and will help us remove our ancient logging redirect.

@Windchild292
Copy link
Contributor Author

I've also sent you a Log4j2.xml file that will let you redirect to console going forwards.

@Windchild292 Windchild292 merged commit 918329e into MegaMek:master Jan 4, 2022
@Windchild292 Windchild292 deleted the dev_Windchild_GlobalDefaultExceptionHandler branch January 4, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(RFE) Enhancement Requests for Enhancement, new features or implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants