Error handling in VBA: how to do it wrong (2 Viewers)

[Josef P.]

Addition to: https://www.access-programmers.co.uk/forums/threads/menus-on-a-form.326893/post-1868023
I'm starting a new thread since this doesn't fit the actual topic of the linked thread.

However, since I always see a similar structure in error handling, I wanted to show an example of how not to implement error handling.

This error handling is incorrect:

Public Sub TestMe()

On Error GoTo ErrHand

   CalcAndSaveResult 2, 1

ExitHere:
Exit Sub

ErrHand:
   HandleError "TestMe"
   Resume ExitHere

End Sub

Private Sub CalcAndSaveResult(ByVal a As Long, ByVal b As Long)

   Dim result As Long
   Dim InsertSql As String

On Error GoTo ErrHand

   result = CalcResult(a, b) '<--- an error occurs in this procedure and an incorrect result is returned.
                             '     From here on there is no error => code continues to run as if nothing had happened.

   InsertSql = "insert into TableABC (a, b, R) Values (" & Str(a) & ", " & Str(b) & ", " & Str(result) & ")"

   MsgBox "Result (a-b)/(b-1) = " & result & vbNewLine & vbNewLine & InsertSql
   ' ^
   ' |.. dummy replacement for data operation e.g:
   ' CurrentDb.Execute InsertSql, dbFailOnError

   ' Save 0 is correct?

ExitHere:
Exit Sub

ErrHand:
    HandleError "CalcAndSaveResult"
    Resume ExitHere

End Sub

Private Function CalcResult(ByVal a As Long, ByVal b As Long) As Long

On Error GoTo ErrHand

    CalcResult = (a - b) \ (b - 1)

ExitHere:
Exit Function

ErrHand:
   HandleError "CalcResult"
   Resume ExitHere

End Function

Private Sub HandleError(ByVal ProcName As String)
   ' dummy replacement for error logging (save in text file, table, ...)
   MsgBox "Error " & Err.Number & " in " & ProcName & ": " & Err.Description
End Sub

Is the error of the error handling recognizable?

The solution is simple:
Either pass the error up or include error handling for unexpected/unresolved errors in the top-level procedure only.

 

[MarkK]

Put error handling wherever you need to.

 

[The_Doc_Man]

With some exceptions, EVERY module should have its own private error handling.

The exceptions relate to subroutines that, of their nature, cannot generate an error, perhaps because they pre-screen the inputs before attempting to execute code that could generate an error. I.e. rather than trap the error, they avoid it. Also, something that only does bitwise logic using AND, OR, NOT, XOR, and EQV (bit-twiddling) cannot generate a math error. There can be other cases to represent the possible exceptions.

Allowing an error to "bubble up" to an upper subroutine layer will sometimes make sense, but more often than not, it merely asks for confusion.

 

[Josef P.]

First of all: please consider the artificially generated error in the example only symbolically. One will catch the division by 0 already completely above.
My point in the example is that the statement that every procedure must have error handling is wrong if the error handling consists only of logging the error and then continuing as if nothing had happened.

If you cannot fix an error in a procedure, you must not ignore it. You can only fix or avoid expected errors in a procedure.
The progress should be stopped in case of an unexpected error, unless it is known in the procedure code that it is possible to continue in case of any error. This should be done in this procedure and not by deleting errors in sub-procedures.

If you write clear procedures (only 1 task per procedure), error handling is usually not very complex.

Error handling is necessary if:
* procedure is the top call level (typical: ACommandbutton_Click or ATextbox_AfterUpdate procedure) ... + possibly error info to User
* data objects (recordsets) or similar are opened and should also be closed (pass error upwards after closing the recordset)
* an expected error is corrected (here nothing must be passed on upward, since the error was eliminated)

Procedure without error handling
* no expected error is known and the procedure is called by other procedures

In summary: Error handling only where it is necessary.

 

[isladogs]

Some time ago, I posted the dummy code below and asked people to state whether all the errors would be triggered.
If not, which would/would not and why

Private Sub cmdErrorTest_Click()

On Error GoTo Err_Handler

 'add your code here
    MsgBox "Click OK to raise error 11 in the main code"
   Err.Raise 11
  
Exit_Handler:
    MsgBox "Click OK to raise error 94 in the Exit Handler section"
    Err.Raise 94
    Exit Sub
  
Err_Handler:
    MsgBox "Error " & Err.Number & " in cmdErrorTest_Click procedure : " & vbCrLf & _
        Err.Description, vbCritical, "Program error"
    MsgBox "Click OK to raise error 7 in the Err Handler section"
    Err.Raise 7
    Resume Exit_Handler

End Sub

 

[The_Doc_Man]

@isladogs - your sample code would raise error 11, which would then raise error 7. However, error 94 would not be raised because error 7 would be raised by an active handler, which would automatically "bump" up to the next call level. (Technically called a "stuck unwind.") The code following label Exit_Handler would not be executed so the Err.Raise 94 will be skipped. Error 11 would be HANDLED by the code after label Err_Handler as shown. Error 7 would be HANDLED by the next error handler available from higher on the call stack. Since we don't see whether there is an OnError entry for the cmdErrorTest_Click() routine's containing form, it is ambiguous as to WHERE error 7 would be handled. If there is no OnError entry for the form in question, the error would be handled by the Access "last chance" error handler, which would give you the message box that includes the "Debug" and "Reset" options - and the "Continue" option will probably be grayed out in that case.

I agree with @Josef P. that having an error handler that ONLY logs an error but does nothing else with it is probably not such a good idea. Though I might disagree to this extent: While developing and debugging, an error handler in every routine DOES help you to identify situations that will need handling. But there are routines that just don't need error handlers, which is why in my earlier comment, I said "with some exceptions." And to the extent that they would help you realize you had a possible error condition in a given routine that you THOUGHT was impregnable, even just an error log can be useful for the developer. For the end user, not so much.

 

[Josef P.]

While developing and debugging, an error handler in every routine DOES help you to identify situations that will need handling.

In development, it doesn't bother me that much because I keep the option "Break on All Errors" enabled.
The callstack is also interesting to me for error reports from the users. This is done by vbWatchdog for me. Without vbWatchdog this is very cumbersome to write in VBA.
Before I used vbWatchdog, it looked something like this (code simplified):

Public Sub TestMe()

On Error GoTo ErrHand
   CalcAndSaveResult 2, 1

ExitHere:
Exit Sub

ErrHand:
   HandleError "TestMe", "TestMe->" & Err.Source
   Resume ExitHere

End Sub

Private Sub CalcAndSaveResult(ByVal a As Long, ByVal b As Long)

   Dim result As Long
   Dim InsertSql As String

On Error GoTo ErrHand

   result = CalcResult(a, b) '<--- err raised
   InsertSql = "insert into TableABC (a, b, R) Values (" & Str(a) & ", " & Str(b) & ", " & Str(result) & ")"
   MsgBox "Result (a-b)/(b-1) = " & result & vbNewLine & vbNewLine & InsertSql

ExitHere:
Exit Sub

ErrHand:
    Err.Raise Err.Number, "CalcAndSaveResult->" & Err.Source, Err.Description

End Sub

Private Function CalcResult(ByVal a As Long, ByVal b As Long) As Long

On Error GoTo ErrHand
    CalcResult = (a - b) \ (b - 1)

ExitHere:
Exit Function

ErrHand:
   Err.Raise Err.Number, "CalcResult->" & Err.Source, Err.Description

End Function

Private Sub HandleError(ByVal Source As String, ByVal SourceTree As String)
   ' dummy replacement for error logging (save in text file, table, ...)
   MsgBox "Error " & Err.Number & " in " & Source & ": " & Err.Description & vbNewLine & vbNewLine & SourceTree
End Sub

... and that would output only the callstack without variable contents, etc.

With a HandleError procedure (with logging etc.) it looked something like this:

...
ExitHere:
   Exit Function

HandleErr:
   Dim ErrNo As Long, ErrSource As String, ErrDesc As String
   Select Case HandleError(ErrNo, ErrSource, ErrDesc, "ProcName", DefaultErrorHandlerMode)
      Case ErrorHandlerMode.ErrExitProc
         Resume ExitHere
      Case ErrorHandlerMode.ErrContinue
         Resume
      Case ErrorHandlerMode.ErrNext
         Resume Next
      Case Else
         Err.Raise ErrNo, ErrSource, ErrDesc
   End Select