Library code snippets

"On Error Resume Next" considered harmful

As any seasoned VB programmer knows, the On Error Resume Next statement is used to check for errors the old (I might also say the C-style) way. When this statement is executed, any runtime error will be silently trapped and stored in the global Err object. We VB-ers typically use this construct to execute some "non-mission critical" code, where errors can be safely ignored. The canonical example I have seen many, many times, is the Form_Resize event handling procedure:

Private Sub Form_Resize()
    On Error Resume Next
    ' Resize the child controls on this form…
End Sub

If this procedure did not contain the On Error Resume Next statement and a runtime error would occur, the application would be terminated with a nasty error message. (You do catch runtime errors in every event handling procedure, don't you?) In cases like this, the On Error Resume Next statement is quite handy, because it means less typing and more compact code. However, there are times, when this "handiness" might be very dangerous. The true danger of the On Error Resume Next statement lies in the fact that it makes it too easy to ignore the runtime errors. I have seen several cases, when ignoring runtime errors unintentionally was a recipe for disaster.

Let me provide you with a real-life example taken from my own experience.

Imagine a customer who had been using my application for several months and was happy with it. The application has a typical (somewhat boring:-) three-tier architecture (in the old days known as the Microsoft Windows DNA):

  • VB6 front end; a typical forms-based application built with several 3rd party controls (GridEX, ActiveReports and AddFlow if you must know).
  • VB6 back end; a DLL component configured to run as a COM+ application implementing the application's business logic (there is also the data access code-who writes middle-tier data access layer anyway:-).
  • A set of T-SQL procedures for retrieving a updating the application's SQL Server 2000 database.
One day the customer requested a new feature. After analyzing the request I realized that the request could be implemented just by enhancing one stored procedure. Great, I thought. I checked out the procedure from SourceSafe, fired up Query Analyzer, changed the procedure, debugged it and tested with the application on our test system. Everything went fine. The next day I visited the customer and deployed the stored procedure on his server. After that, I started the application, invoked the feature that used the new functionality and…the application hung!

It took me two hours until I found out the problem and was able to fix it.

Here is what happened:

When writing stored procedures I always follow this pattern:

if [XXX procedure exists]
    drop procedure XXX
go
create procedure XXX
as begin
    set nocount on
    -- Procedure body goes here
end
go
grant execute on XXX to public
go

This time, however, I forgot to put GO after the stored procedure's body, so the GRANT statement has been part of the procedure itself. When the procedure was run from my test system, my account had the permission to execute the GRANT statement. When it was run at the customer's site, the GRANT statement generated the "Grantor does not have GRANT permission" error. The weird thing was that when the procedure was called from the middle-tier code (using ADO, of course), the call to Command.Execute did not generate runtime error (it would have been trapped and logged at this time) and it produced a seemingly correct recordset. The recordset was then passed to some "non-mission" critical code that looped through all the records and there was the catch. The procedure looked like this:

Private Sub CheckRecords(ByVal dbRS As ADODB.Recordset)
    On Error Resume Next
    Do Until dbRS.EOF
        ' do something - not important here
        dbRS.MoveNext
Loop
End Sub

The real problem was that the MoveNext call generated runtime error that was ignored and the loop never made it to the EOF = True condition. That's it! You can derive many morals from this story, but for me, the most important one is "Never ever use the evil On Error Resume Next statement in your code".

Comments

  1. 02 Jun 2004 at 01:48

    I have a simmiliar experience using ASP. The when there is error when creating recordset, the recordset will loop forever. Interestingly this happends on every machine except one machine. On that machine the error successfully trap. I have been trying to find is there any settings on IIS or SQL Server cause this. Any idea?

  2. 16 Feb 2004 at 07:21

    Hi Palom,
    Thank you very much for the reply. I agree with what you have said.


    - Guhan

  3. 13 Feb 2004 at 08:07

    Hi Guhan,
    Of course, if you'd use "On Error Resume Next" and if you DO check the Err variable after each statement that might generate an error, then yes, there is nothing wrong with that. The problem with this inline approach is (like the article said) that it is just too easy to forget the checking. The structured exception handling using 'Try Catch', while having its own quirks, is far more robust, elegant (and modern;-) that the inline approach.


    Just my $0.2


    Palo

  4. 01 Feb 2004 at 08:08

    I am Guhan from Srilanka.


    I am just nothing compared to your knowledge and experience sir.


    But what I learned in my MCP curriculum is that you use the "On Error Resume Next" statement in a technique called "Inline Error Handling". That is you should use the statement where you think the error would occur and that you should check the error number of the Err object immediately after that statement to determine the status.


    I hope that nothing will go wrong if we keep track of errors, rather than just putting the "On Error Resume Next" on the top of the event handler code.


    I like to know if I am wrong on this matter sir.


    Thanking You,
    Guhan.

  5. 22 Jan 2004 at 19:56

    I think the main problem is that it can easily hide major ones too, without you even realising  


    Fortunately, with VB.NET we can choose to ignore particular errors, and throw others

  6. 21 Jan 2004 at 21:01

    "on error resume next "  is a life saver for minnor runtime errors its saved me plenty of times

  7. 01 Jan 1999 at 00:00

    This thread is for discussions of "On Error Resume Next" considered harmful.

Leave a comment

Sign in or Join us (it's free).

Palo Mraz I live in Slovakia with my wife, two sons (fulltime), one daughter (occasionally) and a dog. I've been doing Microsoft Windows development since 1988; primarily in VB. I'm a big fan of the MS .NET ...
AddThis

Related discussion

Related podcasts

  • Christian Beauclair

    14 mai 2008 (�mission #0074) ::.Christian Beauclair: Stratégies de migration VB6 vers .NET Nous discutons avec Christian Beauclair des stratégies de migration VB6 vers .NET. Entre autres, nous discutons comment utiliser le "VB 6 Code Advisor" et le "Interop Forms Toolkit" pour ajouter la puiss...

We'd love to hear what you think! Submit ideas or give us feedback