69

I've started using try-catch blocks (a bit late, I know!), but now I'm not sure what to do with the exception once I've caught it. What should I do?

Try
    connection.Open()
    Dim sqlCmd As New SqlCommand("do some SQL", connection)
    Dim sqlDa As New SqlDataAdapter(sqlCmd)
    sqlDa.Fill(dt)
Catch ex As SQLException
    ' Ahhhh, what to do now!!!?
Finally
    connection.Close()
End Try
Jaymin
  • 2,879
  • 3
  • 19
  • 35
iamjonesy
  • 24,732
  • 40
  • 139
  • 206

16 Answers16

64

Rule of thumb for exception handling--If you don't know what to do with it, don't catch it.

Corollary to the rule of thumb for exception handling--Handle exceptions at the last responsible moment. If you don't know if this is the last responsible moment, it isn't.

Jaymin
  • 2,879
  • 3
  • 19
  • 35
  • 2
    I disagree with this, and have edited my answer to reflect why. – NibblyPig Apr 08 '10 at 15:57
  • 1
    +1 - and.. If you can't do anything useful about it here, don't catch it here. – Sky Sanders Apr 08 '10 at 19:54
  • @SLC - i think you are mis-interpreting the answer. Of course your application should not blithely crash due to an unhandled exception and an app level handler is usually a good idea. Catching an exception just because you can anticipate one not always the best design. – Sky Sanders Apr 08 '10 at 19:58
  • @Sky - There is a chance your application may crash, if the user performs a certain action, or if there are a certain set of circumstances such as a full hard disk or a lack of network connectivity. You know exactly what will make it crash. You don't choose to catch the exception, even though you can anticipate it. Can you explain _why_ you would not catch it? – NibblyPig Apr 08 '10 at 20:11
  • 2
    @SLC - will and i have already explained exactly why. If you cannot do something useful with an exception where it is generated then there is no reason to catch it THERE. It is a design concern. Libraries are a prime example. Of course if you have exceptions bubbling all the way to the top with no AppDomain.UnhandledException handler then you have another issue entirely. Be clear on this, SLC, i am not suggesting to ignore exceptions. – Sky Sanders Apr 08 '10 at 21:18
  • @SLC Looks like you need to find out about how to handle exceptions and prevent your app crashing with **one** single general exception handler. Check out the documentation http://msdn.microsoft.com/en-us/library/system.windows.forms.application.setunhandledexceptionmode.aspx – MarkJ Apr 09 '10 at 07:55
  • 4
    Then I think we're both confused about what each other is saying. Read your answer again. If you don't know what to do with an exception, don't catch it. So your database might throw an update exception, for one of a million reasons - you're saying if you don't know what to do with that exception, you shouldn't catch it. Nothing could be more wrong than that. Imagine if it throws an sql exception because your anti-viruses is blocking it, something you never predicted. You should catch the exception and display it, letting them continue, NOT ignoring it and letting your program crash and burn. – NibblyPig Apr 09 '10 at 08:25
  • If you want to catch the exception somewhere else, or let it bubble up, _assuming your application can do this_ (which most can't, if you think about it, generally the code you write is not a library/component, there is nothing to bubble TO), then you are **catching the exception** which is exactly the opposite of your answer, so you are contradicting yourself. – NibblyPig Apr 09 '10 at 08:29
  • 2
    Hard disk or network latency, if you are catching an exception or not it is too late anyway. Let the damn thing bubble up the call stack and handle the odd system fault at the top level of the app. – James Westgate Apr 12 '10 at 22:33
  • 4
    Hello reddit. Thanks for all the rep for such a flippant answer. Today, my answer would be slightly different; I'd also include the corollary: Handle exceptions at the last responsible moment. If you don't know if this is the last responsible moment, it isn't. –  Jul 20 '11 at 11:21
24

What do you want to do? It depends entirely on your application.

You could have it retry, you could display a message box to the screen, write to a log, the possibilities are endless.

As a developer, you can't predict every possible error. It's a good idea to have a generic catch code in even if you don't know how to fix the error. You could be saving to a database, and one of 50 database errors might occur, and you won't be able to write code to handle every single one.

You should put a generic catch in to make sure your program doesn't simply crash, and in some cases simply displaying the error and letting them have another go is enough. Imagine if the cause was 'disk is full'. You could merely print out the exception, and let the user try again - they'd know what to do, and solve the problem themselves.

This is why I disagree with the comment about not handling it if you don't know what to do. You should always handle it, even if it is just to display a message box saying 'Error' because it's infinitely better than "This program has performed an illegal operation and will be closed."

Jaymin
  • 2,879
  • 3
  • 19
  • 35
NibblyPig
  • 51,118
  • 72
  • 200
  • 356
  • I would also recommend reducing the amount of code inside your block, especially in the above example. You are a) connecting to a database and b) executing an SQL command, both of which can raise a lot of exceptions. Put each in a separate try catch (or parse your ex/exception) so you have a better idea of what type of error you're dealing with (have you been punched in the eye or been stamped on the foot?) – CResults Apr 08 '10 at 12:48
  • You could also end end the application as the exception is fatal – Victor Hurdugaci Apr 08 '10 at 13:13
  • 1
    @Victor Hurdugaci that exception is not fatal, unless the program requires that data to be running. I always try to not end the program because normally you can let the user try again, try again, let them know something is wrong, let the user know what to do. I don't like programs I've been using to just shutdown. – msarchet Apr 08 '10 at 14:20
  • 13
    "You should always handle [exceptions]". I cannot disagree with you more. Many languages provide a last-chance catchall for exceptions that cannot be handled. You let exceptions you cannot handle or do not know how to handle and show your users a nice "You're F-ed" dialog at this last step. –  Apr 08 '10 at 16:03
  • Can you explain why it's wrong? I don't quite understand your comment. Nothing could be worse than your program crashing with a generic windows error. – NibblyPig Apr 08 '10 at 16:07
  • How do you decide which segments of code should require a try catch? (and avoiding using the block around everything...) – baron Apr 09 '10 at 04:28
  • 3
    @SLC Will means that you can prevent your program crashing by adding **one** single general exception handler: you don't need to put exception handling in **every single routine**. Check out the documentation http://msdn.microsoft.com/en-us/library/system.windows.forms.application.setunhandledexceptionmode.aspx – MarkJ Apr 09 '10 at 07:53
  • 1
    @Baron You can pretty much tell, some code that puts text into a textbox isn't going to break, but code that connects to a database and updates a record, or writes to a logfile, etc. is likely to be prone to breaking. You could have global error catching code, but I can't see how that would be a good thing, especially if some errors can be ignored with just a warning dialog ("Log not written: disk full"). There is no global solution to every error and so relying on catching the exception globally is not a good solution, but more of a failsafe. You should take care of it before it comes to that – NibblyPig Apr 09 '10 at 08:33
  • @SLC I agree with your sentiment that the default behavior of most applications when an unhandled exception is encountered is not very good. I think everyone is saying the same thing. Others are just saying rather than handling every exception by hand, you have a generic uncaught exception handler that fires before the exception gets to the OS. – Nick Spacek Jul 19 '11 at 12:00
  • 1
    I disagree, bad exception handling is worst than no exception handling. What about the principle of fail fast? I understand having generic exception handling in UI or framework code (for example in a thread pool) but you have to be extremely careful not to catch too much. There's nothing worse than API-like code that catches an exception, just logs it (prints it, whatever), and goes merrily without giving the caller an opportunity to deal with it. I feel much more comfortable advising someone to just not handle it if they don't know what to do. – juancn Jul 19 '11 at 18:45
20

It depends on your business logic, but you may want to do one or more of the following.

  • Rollback your transaction
  • Log the error
  • Alert any user of the application
  • Retry the operation
  • Rethrow the exception

You may also wish to check if the exception is of a type you can handle before one of the above.

A good article on VB.NET exception handling is Exception Handling in VB.NET by Rajesh VS.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Colin Pickard
  • 45,724
  • 13
  • 98
  • 148
14

I see a lot of recommendations not to catch it if you don't know what to do with it. That's only sort of right. You should be catching exceptions, but maybe not at this level. Using your code for an example, I'd write it more like this:

Using connection As New SqlConnection("connection string here"), _
      sqlCmd As New SqlCommand("do some SQL", connection), _
      sqlDa As New SqlDataAdapter(sqlCmd)

    sqlDa.Fill(dt)
End Using

Not only is there no Try/Catch/Finally, there's no open or close. The .Fill() function is documented as opening the connection if required, and the Using block will make absolutely certain it's closed correctly, even if an exception is thrown.

This leaves you free to catch exceptions at a higher level — in the UI or business code then calls the function where your query runs, rather than right here at the query itself. This higher-level code is in a much better position to make decisions about how to proceed, what logging needs to happen or show a better error message to the user.

In fact, it's this ability for exceptions to "bubble up" in your code that makes them so valuable. If you always catch an exception right there where it's thrown, then exceptions aren't adding much value beyond the vb's old "On Error Goto X" syntax.

Jaymin
  • 2,879
  • 3
  • 19
  • 35
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
9

It depends on what the SQL does really. Is it something:

  • that the user did? Perhaps create an account or a message - then you need to inform the user that something is wrong
  • that the application does naturally? Like cleaning up logs or something similar? Is it fatal? Can the application go on? Is it something that can be ignored - perhaps retried? Should the administrator be announced?

Start with these questions, they will usually lead to answers about how to treat exceptions

Jaymin
  • 2,879
  • 3
  • 19
  • 35
laura
  • 7,280
  • 4
  • 35
  • 43
5

Unless you were expecting the error (the API says it can occurr) and can handle it (there is an obvious step to take if the exception occurrs), you probably want to crash with a lot of noise. This should happen during testing / debugging, so that the bug can get fixed before the user sees it!

Of course, as commenters have pointed out, the user should never really see all this noise. A high-level try/catch or some other method (EMLAH, I hear for .net web projects) can be used to show the user a nice error message ("Oops! Something went wrong! What on earth were you trying to do?") with a submit button...

So basically, my point is this: Do not catch errors you don't know how to handle! (except for the high-level handler). Crash early and with as much information as possible. This means you should log the call stack and other vital information if at all possible for debugging.

Daren Thomas
  • 67,947
  • 40
  • 154
  • 200
  • 3
    On the other hand, never ever "crash with a lot of noise" before user eyes. Say something like "Ops, something went wrong, we're working on it.". No details to see in public. –  Apr 08 '10 at 12:32
  • 4
    Best to display a custom page with a "please tell us what you were doing" textbox and submit button, so they can vent. Of course, don't hook it up to anything. – NibblyPig Apr 08 '10 at 12:41
2

If don't know how to react to it, don't catch the exception.

Catch it instead in a place where you have the possibility to handle it and where you know how to continue execution afterwards.

sth
  • 222,467
  • 53
  • 283
  • 367
2

One thing to keep in mind is that in sections where you feel you do want to use a try/catch, you should move your Dim statements out of the try block. You can assign them values inside the block, but if you define them inside the try block, you won't have access to those variables inside the catch section as they will be out of scope at that point.

My standard practice in catch blocks, including trying to remediate the error if possible, is to write out to whatever logging mechanism I am using information about key variables that were involved in the section causing the error. It's not necessary, but I've found it often helps with debugging problems.

BBlake
  • 2,388
  • 1
  • 22
  • 31
2

I'd like to suggest to you that if you do not know what to do with an exception, do not catch it. Too often, coders catch exceptions and just swallow them whole.

catch (Exception ex)
{
  /* I don't know what to do.. *gulp!* */
}

Clearly this isn't good, because bad things are happening, and no action is being taken. Only catch exceptions that are actionable!

That said, graceful error handling is important. Don't want your app to crash, right? You may find ELMAH useful for web applications, and a global exception handler is pretty easy to set up for WinForms or XAML desktop apps.

Putting all that together, you might find this strategy useful: 1. catch specific exceptions that you know might occur (DivideByZeroException, SQLException, etc.) and steer away from the generic catch-all Exception; 2. re-raise the exception after you handle it. e.g.

catch (SQLException ex)
{
  /* TODO: Log the error somewhere other than the database... */
  throw; // Rethrow while preserving the stack trace.
}

What can you really do with an SQLException? Did the database connection disappear? Was it a bad query? You probably don't want to add all the handling logic for that, and besides, what if it's something you didn't expect? So just handle the exception if you can, re-raise it unless you're certain it's resolved, and gracefully handle it at a global level (e.g. Show a message like "Something went wrong, but you might be able to continue working. Detailed info: '[ex.Message]'. Abort or Retry?")

HTH!

John Saunders, thanks for the correction.

Jaymin
  • 2,879
  • 3
  • 19
  • 35
Todd Sprang
  • 2,899
  • 2
  • 23
  • 40
  • 2
    Careful logging and re-throwing. If you log at each level, you could end up with many, many entries of the same exception in your error log. Google "log rethrow" for a few tips on the right way to go. – Isabelle Wedin Apr 08 '10 at 13:19
  • @JohnSaunders You should consider deleting your comment now that the answer has been fixed. – Mark Hurd Sep 04 '10 at 06:47
  • @Mark: thanks. I didn't know about the change. Downvote removed. – John Saunders Sep 04 '10 at 06:59
1

First, if there is no way for your application to handle the exception then you shouldn't catch it in the first place (logging can be done via exception event handlers without catching).

If there is something you can do then do that. For example, roll back the transaction and report it failed to the user. This is going to depend on your application.

Community
  • 1
  • 1
jk.
  • 13,817
  • 5
  • 37
  • 50
1

FYI, you don't have to use the catch to have a finally statement.

Sometimes people throw a new exception that is more firendly and add the original exception as the inner exception.

Often this is used to log the error to a file or send an email to an operator.

Sometimes, especially with SQL, it may just mean a duplicate key which means, for example, please choose another username, that one is already taken - it's all down to your application.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
James Westgate
  • 11,306
  • 8
  • 61
  • 68
0

I would consider logging that the exception took place and notify the user that you were unable to complete their request. This assumes that this SQL request is necessary to completion of the action the user is attempting to perform.

Adam Driscoll
  • 9,395
  • 9
  • 61
  • 104
0

It depends entirely on context. Possible options include "Use default data instead of data from the database" and "Display an error message to the user".

Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
0

If you don't want to do anything with it, you can always just do try/finally rather then try/catch/finally

Matt Briggs
  • 41,224
  • 16
  • 95
  • 126
0

Well, That depends primarily if you are making money from your application or not. By my experience, nobody (who paid for an application) likes to see it crashing and closing, instead they prefer a short and explanatory message about the error and a second chance to continue whatever they were doing plus a chance to save/export their modified data. IMO a best practice is to catch everything that can cause something wrong out of your control, like I/O operations, networking related tasks, e.t.c. and display a relevant message to the user. In case that an exception is thrown due to a logic related error is better not to catch it since this will help you to find and correct the actual error.

Hope this helps.

ChD Computers
  • 3,135
  • 3
  • 23
  • 33
0

Depending on the type of application, consider either a global logging handler (i.e. 'Application_Error' for ASP.NET applications) or using a pre-built open source or MSFT provided exception handling framework.

The Exception Handling Application Block as part of the Enterprise Library 5.0 is a suggested framework to use.

Aside from logging, I agree that exceptions need not be explicitly caught unless you need to do something meaningful with them. And the worst mistake is just to 'Catch' and then 'Throw' again as this messes the StackTrace up.

atconway
  • 20,624
  • 30
  • 159
  • 229