[RESOLVED] Any way to know if an exception was thrown while in a finally block?

[Bonesnap]

So after a series of try/catch/catch/catch/etc. blocks the finally block executes regardless if an exception was thrown or not. My question is if there is some way of knowing/detecting if one was thrown. Currently the only thing I can think of is to use some kind of variable as a flag, set it to false by default, but then in each catch block I'd have to include $flag = true; which could get tedious. Seems like there should be an easy way built into the language that does this. Do other languages have this ability?

[laserlight]

Bonesnap wrote:

Currently the only thing I can think of is to use some kind of variable as a flag, set it to false by default, but then in each catch block I'd have to include $flag = true; which could get tedious.

Hmm... I agree that this looks rather tedious:

try {
    $exception_thown = false;
    // ...
} catch (E1 $e) {
    $exception_thrown = true;
    // ...
} catch (E2 $e) {
    $exception_thrown = true;
    // ...
} catch (E3 $e) {
    $exception_thrown = true;
    // ...
} finally {
    if ($exception_thown) {
        foo();
    } else {
        bar();
    }
}

What if you did this instead?

try {
    $exception_thrown = true;
    // ...
    $exception_thrown = false;
} catch (E1 $e) {
    // ...
} catch (E2 $e) {
    // ...
} catch (E3 $e) {
    // ...
} finally {
    if ($exception_thown) {
        foo();
    } else {
        bar();
    }
}

[Derokorian]

Using a flag of some sort is the only way I know of in PHP, Java and C#.

Edit: OTOH, a finally block shouldn't have branching logic, it should be entirely about cleaning up resources IN THEORY. In practice however, nothing matches theory 🙂

[Bonesnap]

laserlight;11053127 wrote:

What if you did this instead?

This is a pretty good idea. I am doing something similar but not quite like this. Neat approach.

Derokorian;11053129 wrote:

Edit: OTOH, a finally block shouldn't have branching logic, it should be entirely about cleaning up resources IN THEORY. In practice however, nothing matches theory 🙂

Heh, true =]

I agree that the finally block shouldn't have branching logic, but I am trying to utilize the finally block to reduce repeated code in the catch blocks.

For example the place where I use the try/catch/finally approach the most is in my AJAX files. If the code is doing something with the database I usually use transactions. If an exception is thrown then I rollback the transaction, which means I have a $db->rollback_transaction(); in my catch blocks. Would be cleaner in my opinion to just write this once in the finally block but only if an exception was thrown.

Similar example would be I have a Log::log_exception(); call in each of my catch blocks to keep a nice debug log. Would be nice if the finally block had scope of the thrown exception and then I could just make one one call.

That is the extent of my branching logic in my finally block. I usually just have a echo json_encode($response); in it, and set all my variables in code with (hopefully) not too much repeated code. laserlight's suggestion should help with that.

A lot of my AJAX files look like this:

$response['response'] = false;

try
{
    $db->begin_transaction();
    // Do or do not. There is no try.
    $db->commit_transaction();

$response['response'] = true;
$response['message'] = 'some kind of success message, not always needed depending on the scenario';
}
catch(Exception1 $e)
{
    $db->rollback_transaction();
    Log::log_exception($db, $e);
    $response['message'] = $e->getMessage();
}
catch(Exception2 $e)
{
    $db->rollback_transaction();
    Log::log_exception($db, $e);
    $response['message'] = $e->getMessage();
}
catch(Exception3 $e)
{
    $db->rollback_transaction();
    Log::log_exception($db, $e);
    $response['message'] = 'Unknown error, check the error log or bug the admin';
}
finally
{
    echo json_encode($response);
}

Just looking for ways to clean it up.

[Derokorian]

Why not catch a more generic exception, and set the message based on type?

try 
{ 
    $db->begin_transaction(); 
    // Do or do not. There is no try. 
    $db->commit_transaction(); 

$response['response'] = true; 
$response['message'] = 'some kind of success message, not always needed depending on the scenario'; 
} 
catch(Exception $e) 
{ 
    $db->rollback_transaction(); 
    Log::log_exception($db, $e); 
    if ($e instanceof Exception1 || $e instanceof Exception2) {
        $response['message'] = $e->getMessage(); 
    } else {
        $response['message'] = 'Unknown error, check the error log or bug the admin'; 
    }
} 
finally 
{ 
    echo json_encode($response); 
}  

[laserlight]

Are these disparate exceptions, or are they derived from some common base class?

Derokorian's looks good, except that I dislike it because it is essentially checking a list of types rather than using polymorphism. On the other hand, I guess the same could be said when you enumerate a list of type in catch blocks.

Also, you might want to consider a form of RAII, limited as it is with PHP: have begin_transaction return a transaction object that is set to rollback when its destructor is called, unless its commit method is called or its rollback method has already been called. This way, even if you forget to explicitly rollback in the face of exceptions, a rollback will happen anyway.

[Bonesnap]

Derokorian;11053143 wrote:

Why not catch a more generic exception, and set the message based on type?

I am not against this idea, but I fear my if() statement could get out of hand as I have found as I am coding more and more using exceptions, I am creating more and more exceptions to keep a fine-grained control of what happens in my code and to be able to respond to the user appropriately. So that list could grow large, though I suppose it would still be cleaner than my current approach.

laserlight;11053145 wrote:

Are these disparate exceptions, or are they derived from some common base class?

Most of them are disparate. I do have three Token exceptions that I usually catch as a Token_Exception, but since that code is usually some of the first code to execute, it always occurs before anything database-related, so I never have a rollback call in that catch block (but I do log the exception).

I'll review my exceptions and see if they could be grouped though they were not created with that in mind. Doesn't mean some refactoring couldn't occur, though.

laserlight;11053145 wrote:

Derokorian's looks good, except that I dislike it because it is essentially checking a list of types rather than using polymorphism. On the other hand, I guess the same could be said when you enumerate a list of type in catch blocks.

I'm not against either method really, though I like the list of catch blocks because to me it gives literal meaning to "catching" an exception as I visualize the exception "falling through" each block until it is caught.

laserlight;11053145 wrote:

Also, you might want to consider a form of RAII, limited as it is with PHP: have begin_transaction return a transaction object that is set to rollback when its destructor is called, unless its commit method is called or its rollback method has already been called. This way, even if you forget to explicitly rollback in the face of exceptions, a rollback will happen anyway.

I have never heard of this. Is this something related to MySQL? What would the transaction object consist of? Lastly, could this be extended to MSSQL? I am using a variation of my DAL with a MSSQL project at work where I am also using transactions.

[Weedpacket]

Bonesnap wrote:

I'll review my exceptions and see if they could be grouped though they were not created with that in mind. Doesn't mean some refactoring couldn't occur, though.

Don't forget that you can catch based on your exceptions' implemented interfaces.

[Bonesnap]

Weedpacket;11053159 wrote:

Don't forget that you can catch based on your exceptions' implemented interfaces.

My exceptions don't have any custom interfaces; they simply extend Exception. :o

[laserlight]

Bonesnap wrote:

I have never heard of this. Is this something related to MySQL?

No, it is just a general way to ensure that resource cleanup (other than memory as handled by garbage collection) happens.

Bonesnap wrote:

What would the transaction object consist of?

Since you only need the database connection to do the commit or rollback, you probably just need that one member variable.

Bonesnap wrote:

Lastly, could this be extended to MSSQL? I am using a variation of my DAL with a MSSQL project at work where I am also using transactions.

Yes.

[Weedpacket]

Bonesnap;11053161 wrote:

My exceptions don't have any custom interfaces; they simply extend Exception. :o

Well, nothing's stopping you from adding interfaces (as part of your refactoring, for instance).

[NogDog]

Hmm...you could go crazy/bleeding-edge and use the runkit extension to actually modify the behavior of the Exception class. 🆒

[Bonesnap]

To add onto this thread while we're discussing this flow, what would be the recommended way to handle issues with the Log::log_exception() call in these catch blocks? We had an instance where the client's security role in MSSQL wasn't correct, so when an exception was thrown it was caught but then tried to write to the log table, which failed and produced a 500 error. The code to write to the log table is pretty basic:

class Log
{
	/**
	 * Logs an exception into the error log database table.
 	 *
 	 * @param Database_Interface $db   The database object.
	 * @param Exception 		 $e    The Exception.
 	 * @param string			 $user The shortname of the user.
 	 *
 	 * @return void
	 */
	public static function log_exception(Database_Interface $db, Exception $e, $user)
	{
	    $query = "	INSERT INTO
	    				error_log (error_exception_type, error_exception_trace, error_exception_user_message, error_line, error_file, error_user)
	    			VALUES (?, ?, ?, ?, ?, ?)";

    $parameters =	[
						['s', get_class($e)],
						['s', $e->getTraceAsString()],
						['s', $e->getMessage()],
						['s', $e->getLine()],
						['s', $e->getFile()],
						['s', $user],
					];

	$db->prepare($query)->bind_parameters($parameters)->execute(false);
}
}

I guess I could throw a try/catch around the database call and then just write to the error log? What about returning a message back to the user? I guess log_exception could return a boolean which could be evaluated? Would love to hear some thoughts on this.

Weedpacket;11053169 wrote:

Well, nothing's stopping you from adding interfaces (as part of your refactoring, for instance).

Heh, very true :p. I haven't had much time to refactor as the client is pushing, pushing, pushing to move forward, so at this point I am picking and choosing my battles and just creating issues in the tracker to revisit later.

[sneakyimp]

Not sure how much flexibility you have in defining your $db object, but you could maybe do a couple of things
make the rollback() method smart enough to determine whether any transaction was even started so if you call this method and no transaction has been initiated, it just fails silently without making a fuss
perhaps wrap some kind of method around execute that doesn't throw an exception but rather returns TRUE or FALSE. then that db log part could look something like this:

    public static function log_exception(Database_Interface $db, Exception $e, $user)
    {
        $query = "    INSERT INTO
                        error_log (error_exception_type, error_exception_trace, error_exception_user_message, error_line, error_file, error_user)
                    VALUES (?, ?, ?, ?, ?, ?)";

    $parameters =    [
                        ['s', get_class($e)],
                        ['s', $e->getTraceAsString()],
                        ['s', $e->getMessage()],
                        ['s', $e->getLine()],
                        ['s', $e->getFile()],
                        ['s', $user],
                    ];

    if (!$db->prepare($query)->bind_parameters($parameters)->execute(false)) {
        // oh dang write the text log instead
        file_put_contents('blah', 'blah');
    }
} 

[Bonesnap]

Thanks for the reply, sneaky!

sneakyimp;11053435 wrote:

* make the rollback() method smart enough to determine whether any transaction was even started so if you call this method and no transaction has been initiated, it just fails silently without making a fuss

I may do something along these lines, but I can't rely on a transaction as not every catch block will need/use one. For example my exceptions regarding tokens are always caught before anything database related can occur, so there's no call to rollback, not to mention I may just be selecting some data so I wouldn't use a transaction in that instance either.

sneakyimp;11053435 wrote:

* perhaps wrap some kind of method around execute that doesn't throw an exception but rather returns TRUE or FALSE. then that db log part could look something like this:

    public static function log_exception(Database_Interface $db, Exception $e, $user)
    {
        $query = "    INSERT INTO
                        error_log (error_exception_type, error_exception_trace, error_exception_user_message, error_line, error_file, error_user)
                    VALUES (?, ?, ?, ?, ?, ?)";

    $parameters =    [
                        ['s', get_class($e)],
                        ['s', $e->getTraceAsString()],
                        ['s', $e->getMessage()],
                        ['s', $e->getLine()],
                        ['s', $e->getFile()],
                        ['s', $user],
                    ];

    if (!$db->prepare($query)->bind_parameters($parameters)->execute(false)) {
        // oh dang write the text log instead
        file_put_contents('blah', 'blah');
    }
} 

I am thinking I am just going to wrap it in its own try/catch block and then if caught simply just write the appropriate data to the error log.

[Bonesnap]

Sorry to bump this thread, but I have actually discovered a way to do this. So the original question was if there's a way to detect if an exception has been thrown in the finally block. And I accidentally discovered that the exception that is thrown is available in the finally block. Meaning you can do a simple isset() check. You would have to make the variables all the same in each catch block (most people just stick with $e anyway), but you have the option to vary them and then check individually:

try {
    // try something
} catch (TokenException $e) {
    // do something
} catch (InvalidArgumentException $e) {
    // do something
} catch (UserException $userException) {
    // do something
} catch (Exception $e) {
    // do something
} finally {
    if(isset($userException) || isset($e)) {
        Log::logException($e);
    }

if(isset($userException)) {
    // something if a user exception is thrown
}

// do other things
}

Pretty neat!