Try-Catch Finally improvement

studio
i_parked

#1

Finally block should be executed always, no matter if exceptions were raised or not and if exceptions were caught or not.
Today, in 2017.1 SP1, if the exception is not caught or if the exception is caught and then rethrown, the finally block is not executed.
Actually, today’s behaviour is like having the activities in the Finally block outside/after of the Try-Catch activity.


#2

Indeed.

That is a Microsoft built activity. If necessary we should re-write it but for now we still have the workaround of moving the finally block outside Try-Catch.


#3

Moving the finally block outside Try-Catch will not fix it, that is actually the current behaviour.

With current behaviour is not possible to have a Try-Finally implementation, useful for the situation when we can’t treat the exception is current workflow, but still we want to execute the finally block.

An workaround will be to have something like following pseudo-code:

var exception;
Try
  <some activities here>
Catch (System.Exception ex)
   exception = ex;
Finally
   <Finally block activities here>
End Try
Throw exception

#4

I agree with @Silviu.

Current workarounds involve basically ignoring how Finally should function, doing a general catch and performing actions before “rethrowing” higher.
It’s workable only if you know how to do it and you know that it actually doesn’t work like you’d expect it to as a programmer. At the very least it should be publicized so that people don’t rely on it and then wonder why their file handles are not released.


#5

Sort of defeats the purpose of a finally right?


#6

I agree with Silviu.
Sometimes we need to try catch all kinds of Exception, and close opened applications in finally block. However, if we don’t rethrow the exception in catch block, we will see the process run successfully on Orchestrator, which is not.


#7

Hello Guys.

Personally I think that the Try-Catch works like expected and in the previously mentioned cases the finally activity should not run.

When the exception is not caught in the try catch that means that the current try catch block should not handle the exception so execution needs to be paused and the error need to be thrown to a higher level. So finally should not be executed. Similarly when an error is Re-thrown.

In both examples you need to have an higher try catch block and the finally from that block should be executed.

Now I think executing the Finally in the specified cases can be more confusing. For instance if you have an uncaught exception by definition you don’t know exactly what happened so you don’t know at that level how to treat it and the finally will still get executed. Normally it might return an error because the system is not in the expected state. If you run the finally afterwards that will return an error and that is what it will be returned to an upper level making debugging more difficult.

But thank you for the suggestion.

Regards,
Cosmin


#8

(emphasis mine)

I completely disagree. The whole purpose of the Finally block is that it’s always executed.
If something should not be always executed, it should not be in a Finally block (or at least it should be conditioned in an If etc. That’s why you see things like if (conn != null) conn.Close/conn.Dispose; [depending on reuse needs]) in the Finally blocks when using is not used.

Consider having a db connection - whatever happens, you always want to make sure connection is closed.
Or a file handle is released, if working on files. Or a socket is closed etc.

One of the base constructions in .Net when working with outside resources is the using clause. It’s whole purpose is to make it easier to make sure that external resources are disposed, which was previously done with try-finally.

Everywhere you look in the MSDN documentations (at least everywhere I was able to find it), there are always sentences like:

Some resource cleanup, such as closing a file, needs to be done even if an exception is thrown. To do this, you can use a finally block. A finally block always executes, regardless of whether an exception is thrown.

Now if we change this predicament, these are some scenarios that you could run into:

  • You read a file as a readwrite stream. You mess something up during this and an exception is thrown (one that is not catched). You have stream closing in Finally (since there’s no using in WF). Stream is never closed, file is now locked. That was the daily transaction file. Before you figure out which robot didn’t release it, SLA’s are in danger.
  • You introduce new operations to an existing db connection. There’s a rare bug that was not diagnosed yet. 10 robots are using this functionality constantly and have long-running jobs. You have conn.close in Finally. You run into a connection limit and all robots fail. Your DBA wants to do nasty things to you.
  • Simpler example - you’re processing PDF’s in a way that they need to be opened (let’s say relative scraping). You open up a PDF, try to do scraping. An unexpected PDF is thrown into the mix (badly formatted, maybe even corrupted). You have KillProcess (because CloseWindow isn’t safe enough) in your Finally clause. That never executes. You have a lingering Acrobat window that messes up reading next PDF’s (wrong window is found etc.).

All of the above can be handled currently in a different way. But all of them would be much easier with a Finally block that executes always.

Yes, and that’s the point. If a lower level workflow does not know how to handle an error in it’s execution, or provide enough information/stabilise so a higher level one could handle it, how a higher level one should know what to do? With proper separation, it shouldn’t even know what the lower level one is doing (only the input/output, rest is an implementation detail).

It’s similar as with Catch reordering - we write in (VB).Net, expecting it to behave like one. But situation that would be a compiler error (unreachable catch) is the way to go. Thing is, noone will figure it out without basically tripping over it, because it’s different to what we were “programmed” to look out for.

With the Catch reordering it’s actually a pretty good change, because it lessens the burden of design. With Finally, it’s the opposite.

This went a little longer, but IMHO, current Finally (I know, it’s MS implementation, w/e) in the Try-Catch-Finally activity does not serve it’s purpose. At all.
It’s basically just another Sequence tacked on at the end.
And this is the core problem - it brings a promise, or an expectation, of working like a tcf block, but doesn’t. Anyone who used any language that has a finally concept will get biten by this unless they by chance find out earlier that it doesn’t work (I was going to say “as expected”, but since it’s the core function of Finally to always execute, let’s call it as it is - it’s not fit for purpose = it doesn’t work).

I think @richarddenton put it exactly right:

I would highly urge you to reconsider or, if you don’t want to reimplement tcf which I can understand, publicize it! Very few people actually know about how this sudo-tcf really works and that’s a huge risk.


#9

In this case the finally follows the vb.net implementation. To be honest I wasn’t familiar with how other languages implement finally (I see that in java it works how you described it) and I see there are use cases were the behavior should be like you described.

I don’t think we can potentially change the Try Catch activity because it’s inherited from Workflow Foundation and there are some backwards comparability problems.

Right now I don’t know how this interaction might be implemented so for now we have the workaround of using two try catches.


#10

Hi guys,

In my opinion, the current implementation is correct, even if it seems weird.

Considering the point of a catch is so you can treat exceptions of type x, that you know how to treat, the point of finally is to do cleanup on the resources used in the try section.
Thus, consider the following example:
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/exceptions/how-to-execute-cleanup-code-using-finally
In it is a classic example of a finally: in case we opened the handle to the file, we will need to close it before we exit the try-catch. Note that since we are respecting the rule of only catch-ing what we can treat, we will not rethrow and so the finally will be executed, allowing us to close the stream to the file and unallocate resources used. Also, since we managed the exception, we want to return the point of execution to the point of call for the try-catch section.

On the opposite spectrum, If we were to only want to log the exception and rethrow, we would place the logging in the catch before the rethrow and not use the finally section.
Since in this second scenario we do not know how to treat the exception, because, essentially, we do not know why it has occurred, it is only natural not to know which resources to clean up, and ignore the finally.

In short, this microsoft implementation of try catch comes with a twist: it’s try, catch, cleanup. It stands to reason cleanup is only executed when and only when you catch.


#11

I like Java implementation of tcf (described by @andrzej.kniola) much better, it gives more flexibility. In case of UiPath, what would be really the difference of moving the code you have in the finally just after the try catch in
case of an unhandled exception? Maybe just some additional scope to some local variables.