fieldrndservices / logger

A Log4j-like library for LabVIEW
http://sine.ni.com/nips/cds/view/p/lang/en/nid/215388
5 stars 2 forks source link

Calling shutdown without a completely configured instance of logger leads to errors #7

Closed DanielAllis closed 3 years ago

DanielAllis commented 3 years ago

In order to ensure there is no stray logger instance active (ensure a known good starting state), I call StopFile and Shutdown before initializing and starting logger during the initialization phase of my application. However, when I do this with the current library I encounter what could be considered unhandled exceptions (or properly thrown errors 😂). This is easy to simulate by calling StopFile & shutdown without an active, configured instance of logger present.

Proposed Changes:

  1. File.lvlib->Manager.vi : "Stop" case modified to check if the root log folder path refnum is "Not a Refnum" before trying to close it.
  2. Unregister.vi : "False" case modified to include Clear Errors.vi with error code 1 cleared when the Listener ID is not found. It doesn't seem like there is a reason to pass the error since a listener ID that isn't in the Listeners queue collection won't need to be unregistered - it already is by default
  3. Release.vi : This vi tries to release all listener queues, but if there are none (i.e. logger uninitialized) then Release Queue will throw error code 1. Adding the Clear Errors.vi with error code 1 cleared ensures this won't error out if the queues don't yet exist.
  4. Shutdown.vi : After the default listener queue is flushed, check if the array of elements is empty. If empty, don't try to write the contents to the log file.
DanielAllis commented 3 years ago

Modified files attached DanielAllis Uninitialized Shutdown Modifications.zip

volks73 commented 3 years ago

Thanks for the bug report and the modified VIs. I will look into integrating your VIs shortly.

DanielAllis commented 3 years ago

FYI - I had added some bookmarked comments in my VIs to make sure I could track down the changes just in case. Those should be pulled out if you want to use as is.

volks73 commented 3 years ago

@DanielAllis I see the bookmarks. A couple of comments,

  1. The VIs you provided are in LabVIEW 2020. The project is currently developed using LabVIEW 2018. I was not able to copy the VIs directly into the codebase/project, but I was able to re-implement in LabVIEW 2018. I think I can eventually bump the development version to LabVIEW 2020, but that is beyond the scope of this issue.
  2. During the re-implementation, I did not see any changes in the Manager.vi, nor bookmark free labels/comments. Were there any modification to the Manager.vi, other than using the modified sub-VIs?
  3. The Specific Error Code functionality of the Clear Errors VI cannot be used as it is not supported in LabVIEW 2010. It is probably time to bump the minimum supported version of LabVIEW from 2010 to 2015 or newer, but I have had users stuck on LabVIEW 2010. This is not really an issue for you or your modifications, but I wanted to make note for historical documentation.
  4. Is it okay to add you (@DanielAllis) to the authors list (Authors.txt) along with your email?
volks73 commented 3 years ago

I feel this is more or less resolved as of 0c1836d10cf7ec0fbd25dbb02756940c80ffb9ba.

@DanielAllis Please let me know if it is okay to add you to the authors list. Once I get confirmation, I will create a new release (v1.10.2) with these changes. An updated VIP will be available here on Github, but it will take a while for it to be available on VIPM and the NI Tools Network.

DanielAllis commented 3 years ago
  1. During the re-implementation, I did not see any changes in the Manager.vi, nor bookmark free labels/comments. Were there any modification to the Manager.vi, other than using the modified sub-VIs?

Sorry - The modification I was referring to is in the Support->File->Manager.vi: "Stop" case. This includes the conditional to avoid trying to close a file that doesn't exist. I've attached my annotated version for your reference.

  1. Is it okay to add you (@DanielAllis) to the authors list (Authors.txt) along with your email?

@volks73 That is perfectly fine.

Missing Edits Logger Issue #7.zip

volks73 commented 3 years ago

A new release v1.10.2 is available that includes all of these modifications, including the Stop case.

An update will be submitted to the NI Tools Network, but it will take some time for it to be available there.