Does my error handling stink or what?

[disruptivehair]

Now that I've added this error handling code, when I export to Excel it flashes up on screen then quickly disappears. If I comment out the new code, the Excel export works fine. What gives? This works in Access.

After I dimmed my variables, I wrote this:

On Error GoTo CreateExcelSH_Err

Then at the bottom of the sub I wrote this:

CreateExcelSH_Exit:
    Call LoadCaption(False)
    For i = 0 To UBound(objRSArray)
        If objRSArray(i).State = 1 Then
            objRSArray(i).Close
        End If
    Next i
    Erase objRSArray
    Erase xlChartArray
    Set xlSheet = Nothing
    Set xlBook = Nothing
        xlApp.DisplayAlerts = False
        xlApp.Quit
        xlApp.DisplayAlerts = True
    Set xlApp = Nothing
    Exit Sub
 
CreateExcelSH_Err:
    Select Case Err.Number
        Case 1004
            MsgBox "Excel cannot draw graphs due to a lack of data.  Please contact the Protection MI team."
            Resume CreateExcelSH_Exit
        Case Else
            MsgBox Err.Number & vbCrLf & Err.Description, vbCritical, "Error!"
            Resume CreateExcelSH_Exit
    End Select

I'm pretty new at error handling so I really have no idea what I'm doing.

[RhinoBull]

I'm not sure which part is new but there is a line tyhat actually closes Excel:

xlApp.Quit

[disruptivehair]

I'm not sure which part is new but there is a line tyhat actually closes Excel:

xlApp.Quit

Yeah, but I told it to do that only when there's an error...at least that's what I thought I told it to do.

If I comment out the error-handling code, the report loads with no errors. If I put it back in it falls over, running under the same conditions.

[si_the_geek]

Am I right in thinking that all of this is for your error handling, and goes after your normal code?

If so, you need to add an "Exit Sub" before it (and I think it would be easier to read if the CreateExcelSH_Exit: section was after the CreateExcelSH_Err: section).

[litlewiki]

Did you try using breakpoints??

[RhinoBull]

If so, you need to add an "Exit Sub" before it .

That is exactly what's missing (I think) - however - I personally have a major problem using labels in the procedure. Those are true way to create spaghetti code...

[si_the_geek]

I don't like labels either - but it's the only way to turn error handling back on for the CreateExcelSH_Exit: part, and that may well be needed.

I have just noticed something else... xlBook is not closed in that section, and it should be (before it is set to Nothing).

[disruptivehair]

I don't like labels either - but it's the only way to turn error handling back on for the CreateExcelSH_Exit: part, and that may well be needed.

I have just noticed something else... xlBook is not closed in that section, and it should be (before it is set to Nothing).

OK...I put the 'Exit Sub' before the error handling and that fixed it. Also added xlBook.Close.

Thanks guys.

[disruptivehair]

That is exactly what's missing (I think) - however - I personally have a major problem using labels in the procedure. Those are true way to create spaghetti code...

That's the only way I know how to do it; how else would I write it unless I used labels? I'm not being defensive, I'm genuinely curious.

[RhinoBull]

Perhaps like this:

Private Sub Command1_Click()
'============================
Dim i As Integer
Dim blnExit As Boolean
 
On Error GoTo ErrHandler
 
    i = 10 / 0 'division by zero error will be raised
    
    If blnExit Then
        MsgBox "Exiting due to error."
    Else
        MsgBox "Exiting normally."
    End If
    
    Exit Sub
 
ErrHandler:
'-----------
 
    MsgBox Err.Description
    
    'do this
    blnExit = True
    Resume Next
    
    'or do all your cleanups directly here
    'and exit when it's done instead of resumming
 
End Sub

[si_the_geek]

My natural instinct is to remove a few lines like this:

Exit Sub
CreateExcelSH_Err:
    Select Case Err.Number
        Case 1004
            MsgBox "Excel cannot draw graphs due to a lack of data.  Please contact the Protection MI team."
'            Resume CreateExcelSH_Exit
        Case Else
            MsgBox Err.Number & vbCrLf & Err.Description, vbCritical, "Error!"
'            Resume CreateExcelSH_Exit
    End Select
   
'CreateExcelSH_Exit:
    Call LoadCaption(False)
...

..but as I mentioned above, this means that error handling will not work on the _Exit section (you will get the standard error messages). This arguably isnt a bad thing, but it depends on whether you think that the _Exit section is likely to have errors.

In this case, I'd say there is a chance of errors, so using the label method would be useful (ideally with a separate error handler for that section to save 'looping', otherwise just an "On Error Resume Next").

[disruptivehair]

My natural instinct is to remove a few lines like this:

Exit Sub
CreateExcelSH_Err:
    Select Case Err.Number
        Case 1004
            MsgBox "Excel cannot draw graphs due to a lack of data.  Please contact the Protection MI team."
'            Resume CreateExcelSH_Exit
        Case Else
            MsgBox Err.Number & vbCrLf & Err.Description, vbCritical, "Error!"
'            Resume CreateExcelSH_Exit
    End Select
   
'CreateExcelSH_Exit:
    Call LoadCaption(False)
...

..but as I mentioned above, this means that error handling will not work on the _Exit section (you will get the standard error messages). This arguably isnt a bad thing, but it depends on whether you think that the _Exit section is likely to have errors.

In this case, I'd say there is a chance of errors, so using the label method would be useful (ideally with a separate error handler for that section to save 'looping', otherwise just an "On Error Resume Next").

Where could an error happen in the _Exit section?

[disruptivehair]

Perhaps like this:

Private Sub Command1_Click()
'============================
Dim i As Integer
Dim blnExit As Boolean
 
On Error GoTo [B]ErrHandler[/B]
 
    i = 10 / 0 'division by zero error will be raised
    
    If blnExit Then
        MsgBox "Exiting due to error."
    Else
        MsgBox "Exiting normally."
    End If
    
    Exit Sub
 
ErrHandler:
'-----------
 
    MsgBox Err.Description
    
    'do this
    blnExit = True
    Resume Next
    
    'or do all your cleanups directly here
    'and exit when it's done instead of resumming
 
End Sub

Isn't that a label though?

[si_the_geek]

Where could an error happen in the _Exit section?

Well two possible places are: objRSArray(i).Close and xlApp.Quit

Isn't that a label though?

Only for the error handler itself (there is no other option), the extra _Exit label is not needed.

[disruptivehair]

Well two possible places are: objRSArray(i).Close and xlApp.Quit

Only for the error handler itself (there is no other option), the extra _Exit label is not needed.

But wouldn't checking if the recordset was open prior to closing it sort of rule out an error there? You're right about the xlapp.quit being a possible failure point but I don't see how the recordsets could cause problems unless something really weird happens...maybe...hopefully...

I'm not getting the distinction between labels and things here; I think I probably need to read up more on error handling before diving back in.

[si_the_geek]

But wouldn't checking if the recordset was open prior to closing it sort of rule out an error there?

Possibly not! It depends what state it is in.. if I remember correctly, if you are editing a record at the time then it wont want to close.

I'm not getting the distinction between labels and things here;

In your original code you had two labels: "CreateExcelSH_Err:" and "CreateExcelSH_Exit:". Only the _Err one is actually required (to enable the error handling), but the _Exit one is useful in situations like this - where you dont want to return to the 'main' code (as RhinoBulls example does), and your clean-up code may cause errors itself.

[disruptivehair]

Possibly not! It depends what state it is in.. if I remember correctly, if you are editing a record at the time then it wont want to close.

Good point, but I never edit records in those subs...the recordsets are opened as read only anyway.

In your original code you had two labels: "CreateExcelSH_Err:" and "CreateExcelSH_Exit:". Only the _Err one is actually required (to enable the error handling), but the _Exit one is useful in situations like this - where you dont want to return to the 'main' code (as RhinoBulls example does), and your clean-up code may cause errors itself.

[/quote]

OK...I think I'll read up more on error handling anyway, it was a weak spot for me in VBA and continues to be in VB.

[si_the_geek]

I'll see if I can explain this a bit more clearly.

When an error occurs in your 'main' code (after the On Error GoTo..), execution jumps to the label you specified (in this case, CreateExcelSH_Err: ). At that point, you deal with the error however you want (in this case, showing an appropriate message).

There are basically two options of what to do next - either return to the main code, or do a clean up.

If you want to return to the main code, there are two main options:

• One is "Resume", which goes back to re-try the line that had the error (which may have the same error again - RhinoBull's example would).
• The other is "Resume Next" as RhinoBull showed, which goes to the line after the one that caused the error. You will get past the error, but depending on what the line of code does, this may cause other errors later (if it was opening xlBook, any later code that uses xlBook will fail).

..both of these re-enable the error handler, so any further errors that occur will also be handled.

If you are doing a clean up, there are two main options again:

• Simply put the clean-up code after the error handler (as in Post #11). Doing this means that the error handler will no longer work - so if the clean-up code has errors, your program will crash.
• Use "Resume Label" to jump to somewhere else (as you had originally). As with the options above this re-enables the error handler, but there is a problem... any errors that occur in the clean-up will mean that the error handler (and thus the clean-up) will repeat.

To get around this 'loop' issue (if you think your clean-up might have errors) you should either use a separate error handler for it, or disable error handling for it (using "On Error Resume Next").


What method you should use depends on the code in the sub/function.. in some cases errors are fixable (eg: if you get a "path not found" you could create the folder and try again), but in others any kind of failure will lead to more errors later (such as the opening of xlBook).