-
Notifications
You must be signed in to change notification settings - Fork 1
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
Error Report Information #9
Comments
This is way too good to give some feedback. Approved. :) |
Thank you for that! However, I did notice something missing: Northwind Traders information. I figure that it would be nice to know which Edition, Version, and Build the error happened in, so that we'd know if the error had possibly been fixed already in a later build than the one the error is occurring in, so I took the liberty of adding that information, as well. Here's the sample:
FYI: I have set this reporting feature up to give the file the name of the class, followed by the word "class", and the timestamp of the error (for uniqueness). For example, the name of the generated file for the above error is: If you think this improvement is good and that this error report will work, then feel free to close this Issue. Thanks, Jiri! |
That's a marvellous idea. I would then suggest to also include a list of all Northwind modules that are plugged in. I am working on a proof of concept on a private branch which I plan to push to the Git repository for your review by the end of the week at the latest if things go fine as so far. |
Excellent idea. I'm looking forward to seeing your proof of concept! I will actually be at home this weekend, also, so I will be able to pull the repository down to my Linux system and get a feel for it on that OS as well. |
OK. I have modified the
As you can see by the entry ArrayList<String> modules = new ArrayList();
ModulesManager.getDefault().loadModules();
Iterator<Module> mods = ModulesManager.getDefault().getModules();
while (mods.hasNext() ) {
String e = mods.next().toString();
modules.add(e);
} With that, I added the following method to your @Override
public String toString() {
return this.name + "[ v. " + version + " ]";
} To get the module names into the error file, I simply took them into the src.append("\nInstalled Modules:");
for (Object module : modules) {
src.append("\n\t");
src.append(module.toString());
} Even though the Let me know what you think. I'm going to go ahead and push the modifications up to the repository and call it a night. |
As explained in my last comment on Issue #9, I have updated the `Logger.critical` method to include the list of installed modules in the error log. I also added a `manifest.xml` file to the default package of all modules except for `com_pekinsoft_northwind`, as it only contains the `Startup` class. Also, @jkovalsy, there is a logical issue with the following code: ```java // Let's customize the main menu bar by loaded modules while (modules.hasNext()) { Module module = modules.next(); MenuProvider menuProvider = (MenuProvider) module.getInstanceOf("com.pekinsoft.northwind.basic.MenuProvider"); jMenuBar1.add(menuProvider.getMenu(this.MainDesktop)); } ``` The problem is that, with the other modules in the `${HOME}/.northwind/modules` folder, `NullPointerException`s are thrown if the module does not contain a `menuProvider` object. Since that is your block of code, I thought that I would let you know this and, perhaps, you can add a check for the `menuProvider` within this block. I am calling it a night. Tomorrow, I will move the `Module` and `ModulesManager` classes back to the `com_pekinsoft_northwind_basic` module, and remove the `com_pekinsoft_northwind_basic_modules` module.
I tested the error reporting that generates a detailed report of the error in the
.northwind/var/err
folder in the user's home folder. The generated report looks like the sample below. Please, look it over and give me suggestions of any other information that should be gathered into it. Thanks for your input!Sample error report:
The text was updated successfully, but these errors were encountered: