Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What happens when code in a Finally block throws an Exception?

Tags:

vb.net

I have a simple block of legacy code that sits inside of a loop that crawls through some potentially bad xml node by node and which needs to be refactored as it is not working as intended:

Try
   xmlFrag.LoadXml("<temproot>" & strXMLfragment & "</temproot>")
   writer.WriteRaw(strXMLfragment)
Catch ex As Exception
   InvalidXML = True
End Try

What this block is meant to do is check for valid xml and then write the xml out. What it actually does is check for invalid xml and then write the xml out only if it is valid. So it needs to be fixed to work as intended.

My first attempt at a fix:

Try
   xmlFrag.LoadXml("<temproot>" & strXMLfragment & "</temproot>")
   'writer.WriteRaw(strXMLfragment)
Catch ex As Exception
   InvalidXML = True
Finally
   writer.WriteRaw(strXMLfragment)
End Try

This works on my test data but I am concerned that WriteRaw may throw an exception on other data. I haven't found a conclusive statement about what will cause WriteRaw to throw an exception and what will happen when code in a Finally block throws an exception.

So I tried rewriting it like this:

Try
   xmlFrag.LoadXml("<temproot>" & strXMLfragment & "</temproot>")
Catch ex As Exception
   InvalidXML = True
End Try
Try
   writer.WriteRaw(strXMLfragment)
Catch
End Try

Frankly it looks ugly as hell. Is there a more elegant way to refactor this or will the first attempt be suitable?

like image 530
Robin G Brown Avatar asked Feb 13 '23 23:02

Robin G Brown


2 Answers

When an excpetion is raised in a Finally block, nothing special happens: the exception is propagated out and up like any other exception, and code after the exception in this finally block will not be executed.

Your first attempt will fail if strXMLfragment is null or an empty string (or due to a already running asynchronous operation).

So if you really want to handle/swallow all exceptions, you'll have to use two Try blocks.

like image 179
sloth Avatar answered Feb 23 '23 02:02

sloth


To make it cleaner, you might want to pull your first Try/Catch into it's own private function and make it reusable:

Private Function TryParseXml(ByVal xml as String) as Boolean
    Try
        XDocument.Parse(xml)
        Return True
    Catch ex As Exception
        Return False
    End Try
End Function

Then wrap your writer.WriteRaw call in it's own Try/Catch.

Dim myXml = "<temproot>" & strXMLfragment & "</temproot>"
If TryParseXml(myXml) Then
    xmlFrag.LoadXml(myXml)
Else
    Try
        writer.WriteRaw(strXMLfragment)
    Catch ex as Exception
        ' handle exception
    End Try
End If

Yes, ultimately this is using two Try/Catch blocks. There is no real way around this as the only real way to determine if Xml is valid is to attempt to parse it and wait for it to blow up.

like image 36
Aaron Palmer Avatar answered Feb 23 '23 04:02

Aaron Palmer