Exception:MailAlerts: Error accessing radon_st

[rkoshak]

Looking through the logs I'm seeing the following error every time a record is pulled from a device:

2023-05-04 10:44:11 -      __main__[  DEBUG] - MailAlerts: Error accessing basement:radon_st: 'radon_st'
Traceback (most recent call last):
  File "/srv/waveplus_bridge/waveplus_bridge.py", line 448, in check_levels
    value = data[source[0]][source[1]]
KeyError: 'radon_st'
2023-05-04 10:44:11 -      __main__[  DEBUG] - MailAlerts: Error accessing basement:radon_st: 'radon_st'
Traceback (most recent call last):
  File "/srv/waveplus_bridge/waveplus_bridge.py", line 448, in check_levels
    value = data[source[0]][source[1]]
KeyError: 'radon_st'
2023-05-04 10:44:11 -      __main__[  ERROR] - Failed to trigger alerts: MailAlerts: Error accessing basement:radon_st: 'radon_st'
2023-05-04 10:44:11 -      __main__[  DEBUG] -   Stack trace:
Traceback (most recent call last):
  File "/srv/waveplus_bridge/waveplus_bridge.py", line 763, in main
    actions.check_levels(sensor_data_no_ts)
  File "/srv/waveplus_bridge/waveplus_bridge.py", line 455, in check_levels
    raise Exception(error_msg)
Exception: MailAlerts: Error accessing basement:radon_st: 'radon_st'

The "basement" device isn't being detected at the time so there is no record. I think what needs to be done is to skip calling "check_levels" when the connection to the device failed and there is no record.

It doesn't seem to disrupt the overall use of the software but it would be nice if these errors didn't get generated in this case.

I don't know the full impacts of this but I think just moving line 746-749 to before the except so that when there is no data it doesn't try to save or analyze it would be the thing. But I don't know this code that well. Maybe just add a check in the MailAlerts to exit quietly when there's no data to analyze.

[Drolla]

Hello, This is indeed a nasty log entry; I have them also sometimes if a device isn't being detected. Your proposed fix would not work, because the check_level method of the Actions instance will still fail since the data isn't available. I think it would be good to have a single-line error or warning in the log when a level check cannot be performed because the data is not available. But there is certainly no need to have a stack trace in this situation. So what do you think, would it be OK to have the following message in case a device is not available?

2023-05-04 10:44:11 -   __main__[WARNING] - MailAlerts: Data not available for basement:radon_st; level cannot be checked!

[Drolla]

Branch mailalert-exception-handling-improvement has been created with the following change: MailAlert.check_levels raises still an exception if some data is not accessible, but do not generate a stack trace. The main loop logs the raised exception as a single line message (level=Error). Optionally a stack trace can be logged if the logging level is set to 'Debug', but this level should only be enabled for debugging purposes. Generally I recommend to set the debug levels of the different modules to 'Info' or 'Error'.

[rkoshak]

A single line warning level log seems appropriate. Is it really an error in this case? An error is already logged that the device isn't responding so it seems like logging another error level is redundant. Or are there cases where this error will occur when we haven't lost the connection to the device?

[Drolla]

You are right, it is more a warning that can be hidden if the logging level is set to Error. The code has been modified in this sense. An error will still be raised in case another failure happens that is not related to the non-availability of device data.

[Drolla]

Took the opportunity to reduce the logging level to a warning in case the 4 attempts to connect to a WavePlus device are failing. Only if all attempts are failing an error is generated.

[Drolla]

Merged branch mailalert-exception-handling-improvement to the master branch (pull request #19). The request has therefore been addressed.