6
\$\begingroup\$

The idea is to make an error handling procedure, which prints messages in DEV mode and shows MsgBox in PROD mode. Looking for any wise idea to improve it:

Public Const SET_IN_PRODUCTION = False

Sub TestMe()

    On Error GoTo TestMe_Error
    Debug.Print 0 / 0
    Exit Sub

TestMe_Error:
    StandardError Err, "TestMe"

End Sub


Public Sub StandardError(myError As Object, Optional moduleName As String = "")

    If SET_IN_PRODUCTION Then
        MsgBox "Error in " & moduleName & vbCrLf & myError.Description, vbCritical, title:=myError.Number
    Else
        Debug.Print "Error in "; moduleName & vbCrLf & myError.Description
    End If

End Sub

The lines StandardErr Err are to be used for almost any procedure and function in the code, for "advanced" error handling.

\$\endgroup\$
5
  • \$\begingroup\$ Looks like your error handler needs an error handler. \$\endgroup\$
    – HackSlash
    Aug 27 '20 at 19:50
  • \$\begingroup\$ @HackSlash - why, what can go wrong there once the code compiles? \$\endgroup\$
    – Vityata
    Aug 27 '20 at 21:13
  • \$\begingroup\$ myError, defined as an object could be null or something other than an error object. \$\endgroup\$
    – HackSlash
    Aug 28 '20 at 15:25
  • \$\begingroup\$ @HackSlash - how can this happen, considering the fact that it is always called with the explicit example? \$\endgroup\$
    – Vityata
    Aug 28 '20 at 15:34
  • 1
    \$\begingroup\$ Make no assumptions about how your code will be used in the future. \$\endgroup\$
    – HackSlash
    Aug 28 '20 at 15:35
4
\$\begingroup\$

Passing around the Error object like this feels intuitively like a good way of freezing error state and providing it to logging/error reporting functions, so I see why you've come up with this.

Reading the comments, @HackSlash does make a good point - that myError could hold anything if it's declared As Object, and that could lead to errors when you try to get its Description or Number. I'd probably change the signature from this:

Public Sub StandardError(myError As Object, Optional moduleName As String = "")

... to this

Public Sub StandardError(ByVal myError As VBA.ErrObject, Optional ByVal moduleName As String = "")
    If myError Is Nothing Then Err.Raise 5, Description:="Must pass valid Err Object"

ByVal because we don't need to change what the caller's copy of the variable points to

By declaring myError with the type (interface) we want, it means rather than receiving the generic and somewhat obscure error message:

Run-Time Error 438

Object doesn't support this property or method

... or even harder to diagnose if Nothing is passed;

Run-Time Error 91

Object variable or With block variable not set

... we get a clearer Type Mismatch error that takes us right to the source

Debug View

The calling site is pretty simple though, so it's unlikely you'd type it in wrong, but still, doesn't harm to add some Error checking. But come to think of it, what does happen if an Error is raised inside the error handling routine? Or If I call Err.Clear? Doing this:

Public Sub StandardError(ByVal myError As ErrObject, Optional moduleName As String = "")

    Err.Clear    'do something to modify GLOBAL error state
    
    If SET_IN_PRODUCTION Then
        MsgBox "Error in " & moduleName & vbCrLf & myError.Description, vbCritical, Title:=myError.Number
    Else
        Debug.Print "Error in "; moduleName & vbCrLf & myError.Description
    End If

End Sub

... results in no Error being printed - myError.Description has been cleared even though I never touched myError explicitly, something funny is going on here...


It turns out Err is not some locally scoped variable that gets supplied as a hidden argument to every Sub or Function, and which you can just pass around. If we look in the object browser it is in fact a function in the VBA.Information module that returns an object of type VBA.ErrObject, but crucially, this is the same object every time. Or in other words (taken from an answer on SO):

When you encapsulate an ErrObject like you did, you're essentially just giving yourself another way (besides the Err function) to access the ErrObject instance - but it's still the exact same object holding the properties of the current run-time error state.

What this means is that you never need to pass the "Err Object" to the logging routine at all, since the Err function will provide access already. What this also means is that you need to be very careful about the possibility of errors creeping into your logging function, since even if these are ignored, they will reset the global error state and your logger could now misreport the errors.

Admittedly, your logger is very simple, so this doesn't present any immediate problem, but what if in the future you want to log to a file:

Private Function GetFile(ByVal path As String) As File

   Dim result As File

   'Try open log file, catch error if it doesn't exist
   On Error Resume Next
   Set result = File.Open(path)
   On Error Goto 0

   'If log file didn't exist, make a new one 
   If result = Nothing Then Set result = File.NewFile(path)

   Set GetFile = result
End Function

Public Sub StandardError(Optional moduleName As String = "")

    Set myFile = GetFile("C:/Users/blah/log.txt") 'Ooops, just nuked global error state

    If SET_IN_PRODUCTION Then
        MsgBox "Error in " & moduleName & vbCrLf & Err.Description, vbCritical, title:=myError.Number
    Else
        myFile.Print "Error in "; moduleName & vbCrLf & Err.Description
    End If

End Sub

o.k that's just pseudocode, I know myFile.Print is invalid syntax, but you get the idea

As Mathieu points out in that post I linked, you can roll your own class that saves runtime error state if you want to be safe, but even that has drawbacks. I find the best way to tackle this is to pass the relevant bits to a generic logger like I did here:

logError "WinAPI.SetTimer", Err.Number, Err.Description

and freeze the Err state precisely when you want to.


#Conditional Compilation

Public Const SET_IN_PRODUCTION = False

I'd probably use conditional compilation here

#If SET_IN_PRODUCTION Then
    MsgBox "Error in " & moduleName & vbCrLf & myError.Description, vbCritical, Title:=myError.Number
#Else
    Debug.Print "Error in "; moduleName & vbCrLf & myError.Description
#End If

and define SET_IN_PRODUCTION either in the project's settings or with code

#Const SET_IN_PRODUCTION = False

simply because conditional compilation is quite a standard method for Debug vs Ship mode, and it replaces a runtime If statement called many times with a compile time one called only once

\$\endgroup\$
2
  • 2
    \$\begingroup\$ I don't know if it's just me, but the link in your post "Or in other words (taken from an answer on SO):" is broken for me and doesn't go anywhere \$\endgroup\$ Aug 31 '20 at 14:50
  • \$\begingroup\$ @HaydenMoss oh yeah sorry, fixed now \$\endgroup\$
    – Greedo
    Aug 31 '20 at 20:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.