Using finally exception block when Slim is being used?

[NotionCommotion]

Why isn't $e set in the finally block?  You might say it is.  Please read on...

 

try {
    $pdo->beginTransaction();
    //insertSomeRecords();
    //Just used to fake a PDOException
    $pdo->prepare('SELECT x FROM invalid_table');
    return true;
} catch (PDOException $e) {
    //Some logic goes here
    throw true?$e: new Exception;
} finally {
    //Why isn't $e set?
    syslog(LOG_INFO,isset($e)?'$e is set':'$e is not set');
    if (isset($e) || !$pdo->commit()) {
        syslog(LOG_INFO,'rollBack');
        $pdo->rollBack();
    }
}

I happen to be using Slim, and I think Slim is somehow catching the exception and somehow resolving it before getting to the finally block, and thus I inappropriately commit the transaction.  Can I use a finally block when using Slim?  If so, how?

 

On a side note, am I doing error handling correctly as shown below?  Two specific areas I am confused with are whether I should use $c['response']->withJson() or $response->withJson()and how $methods is passed to the handler.

 

Thank you

<?php
$c = new \Slim\Container([]);


$c['errorHandler'] = function($c){
    return function ($request, $response, $e) use ($c){
        syslog(LOG_ERR,'ERROR in '.$e->getFile()." (".$e->getline().'): '.$e->getMessage());
        if ($e instanceof NotFoundException){
            return $c['notFoundHandler']($request, $response);
        }
        elseif ($e instanceof NotAllowedException){
            return $c['notAllowedHandler']($request, $response);
        }
        elseif ($e instanceof PhpErrorException){
            return $c['phpErrorHandler']($request, $response);
        }
        else {
            return $c['applicationErrorHandler']($request, $response, $e);
        }
    };
};


$c['notFoundHandler'] = function ($c) {
    return function ($request, $response) use ($c) {
        // return $c['response']->withJson(['error'=>'Page not found'],404);
        // Should I use $c['response'] or $response
        return $response->withJson(['error'=>'Page not found'],404);
    };
};
$c['notAllowedHandler'] = function ($c) {
    //Where is $methods coming from?
    return function ($request, $response, $methods) use ($c) {
        return $response
        ->withHeader('Allow', implode(', ', $methods))
        ->withJson(['error'=>'Method must be one of: ' . implode(', ', $methods)],405);
    };
};
$c['phpErrorHandler'] = function ($c) {
    return function ($request, $response) use ($c) {
        return $c['response']->withJson(['error'=>'Critical System Error'],500);
    };
};
$c['applicationErrorHandler'] = function ($c) {
    return function ($request, $response, $e) use ($c) {
        return $response->withJson(['error'=>$e->getMessage()],422);
    };
};


$app = new \Slim\App($c);


$app->post('/test', function (Request $request, Response $response) {
    $rs=someFunctionWhichEitherReturnsArrayOrThrowsException();
    return $response->withJson($rs[0],$rs[1]);
});


$app->options('/{routes:.+}', function ($request, $response) {
    return $response;
});


$app->run();

 

[NotionCommotion]

I was partially mistaken.  $e is set in the finally block for a PDOException, but not for any other exceptions.

 

Should I just catch Exception after I catch PDOException, and just throw the exception so it is defined?

try {
    $pdo->beginTransaction();
    //$pdo->prepare('SELECT x FROM invalid_table'); //finally block will have $e set
    throw new Exception();
    return true;
} catch (PDOException $e) {
    //Some logic goes here
    throw true?$e: new Exception;
} catch (Exception $e) {
    throw $e;   //Just done to define $e?
} finally {
    //$e set for PDOException but not Exception?
    syslog(LOG_INFO,isset($e)?'$e is set':'$e is not set');
    if (isset($e) || !$pdo->commit()) {
        syslog(LOG_INFO,'rollBack');
        $pdo->rollBack();
    }
}

 

[requinix]

Do you actually need $e for something? I used it in that code as a way to check for success (it could have been buggy) so you could use a flag to the same end.

$success = false;
try {
	$pdo->beginTransaction();
	throw new Exception();
	return $success = true;
} catch (PDOException $e) {
	// ???
} finally {
	if (!$success || !$pdo->commit()) {
		// ...
	}
}

[NotionCommotion]

Do you actually need $e for something? I used it in that code as a way to check for success (it could have been buggy) so you could use a flag to the same end.

 

No I don't.  Thanks, a flag works fine.

 

I am actually returning some data, not true, so will do the following.

            $success=true;
            return [$d,200];

[requinix]

FYI since non-empty arrays are truthy,

return $success = [$d, 200];

would work too.

[kicken]

Your call to $pdo->commit() should be in your try block, not the finally block.

 

try {
    $pdo->beginTransaction();
    //insertSomeRecords();
    //Just used to fake a PDOException
    $pdo->prepare('SELECT x FROM invalid_table');
    $pdo->commit();
} catch (PDOException $e) {
    //Some logic goes here
    throw $e;
}

If an exception is thrown the call to commit will be skipped and the transaction will be automatically rolled back.

[NotionCommotion]

Your call to $pdo->commit() should be in your try block, not the finally block.

 

 

The finally block will always be executed, right?  Then wouldn't the following always either commit or rollback?

finally {
    if (!$success || !$pdo->commit()) {
        $pdo->rollBack();
    }
}

[requinix]

Looking at what I did there (being the author of that code), it is a bit odd: normally one would think of the try{} being the normal execution part, the catch{} as the error recovery part, and the finally{} as a optional (if needed) clean-up part. My thinking at the time was probably along the lines of using the try{} for the bulk of the work, the catch{} for altering the work to adapt to an error, and the finally{} as finalizing the work being done (be that a commit or a rollback).

 

The latter makes sense to me, but the former is probably better as a rule of thumb. While the code is functional (...right?) I suggest following kicken's advice.

[NotionCommotion]

The reason for using the finally block was that I was catching more than one exception it it slightly reduced the amount of script.  That being said, I agree with you who agrees with kicken which agrees with the way I was originally doing it