Page 1 of 2

Exception handling

Posted: Wed Feb 04, 2009 9:01 pm
by rrcatto
I've recently written a mailing list class in PHP 5 which uses Swiftmailer 3.3.3 and quickly discovered that I needed to catch exceptions to prevent my script from fatally crashing, which is not something an end user wants to see.

However, in 3.3.3, you do not use codes for exceptions, only error descriptions, many of them lengthy (and subject to change in future versions), so it's not possible to handle exceptions on a case by case basis, as your code stands.

To handle exceptions, I resorted to hacking your 3.3.3 code to insert exception codes for the exceptions I was experiencing.

Although, I downloaded your 4.0 beta source and had a quick look round, I haven't yet discovered whether you have implemented a standard set of defined codes for each category of exception or not.

If not, I urge you to do so, because without that feature, it is not possible to easily handle exceptions.

I also can't find any documentation yet on exception handling in 4.0.

I intend releasing my mailing list class (and associated files) as open source software as soon as I am satisfied that it is ready for general usage.

Re: Exception handling

Posted: Wed Feb 04, 2009 9:45 pm
by Chris Corbyn
I use different types of Exception, not different error codes:

Code: Select all

try {
  //stuff
} catch (SomeException $e) {
}
catch (DifferentException $e) {
}
This should be sufficient if you're not just doing:

Code: Select all

try {
  // stuff
} catch (Exception $e) {
}
Apologies for v4 documentation. It's not yet ready for release and the documentation is not yet finished ;) I have a chapter I need to write to do with error handling and debugging.

Re: Exception handling

Posted: Wed Feb 04, 2009 9:48 pm
by John Cartwright
If I understood you correctly, you are determining the type of exception by examining it's message? The proper way to detect the various exceptions are (in v3)

Code: Select all

try {
   //swift code
} catch (Swift_ConnectionException $e) {
   //error connecting
} catch (Swift_BadResponseException $e) {
  //error in response
} catch (Swift_Exception $e) {
  //catch rest
}
I haven't looked into exception handling in v4 yet though.

Re: Exception handling

Posted: Thu Feb 05, 2009 1:13 am
by Chris Corbyn
Jcart wrote:I haven't looked into exception handling in v4 yet though.
Same concept, slightly different names:

Swift_SwiftException
Swift_RfcComplianceException extends Swift_SwiftException <-- MIME level stuff that you're not likely to ever see if not modifying it
Swift_IoException extends Swift_SwiftException
Swift_Transport_TransportException extends Swift_IoException <--- Errors in the transport layer (no concept of "responses" in v4)
Swift_DependencyException extends Swift_SwiftException <--- Really should never been seen unless you fiddle with the DI config

NOTE: Swift_Transport_TransportException is ticketed to be renamed to Swift_TransportException before 4.0.0 final.
EDIT | I just knocked over that ticket for the rename so 4.0.0-b5 will use this name.

I was supposed to be out on a hot date tonight, but given that plan fell through ( :cry: ) I think I'll release beta-5 and continue with docs since the docs have not been touched for a while.

PS: If anybody has DITA Open Toolkit Customization experience and would like to help out with the Swift Mailer documentation generation, please contact me.

Re: Exception handling

Posted: Thu Feb 05, 2009 4:10 am
by rrcatto
Chris Corbyn wrote:I use different types of Exception, not different error codes:
But not consistently.

One exception I got from the 3.3.3 library was generated using the PHP base class Exception. This was for a bad email address.

The other was generated using the inherited class Swift_BadResponseException for an SMTP error.

In your send function in Swift.php you create two different kinds of exceptions, both using the same PHP base class Exception. One for a bad recipient address and another for a bad sender address, neither with a code attached, so without hacking your code, it is impossible to tell them apart unless you compare error strings, which is very inefficient and a poor coding practice.

Unless you are doing something unique when you extend the PHP base class Exception, I think it's inefficient to create a different inherited Exception class for each type of exception, which does not alter in anyway the way the base class works, in lieu of the simpler method of passing a unique code per unique exception to the base Exception class. That's the reason the base class accepts both an error message and a code.

You can define all the SWIFT_EXCEPTION_CODES in a separate file, so that anyone using your library can change them to whatever they like to avoid code conflicts with another library, if that should arise.

Re: Exception handling

Posted: Thu Feb 05, 2009 4:38 am
by Chris Corbyn
rrcatto wrote:
Chris Corbyn wrote:I use different types of Exception, not different error codes:
But not consistently.

One exception I got from the 3.3.3 library was generated using the PHP base class Exception. This was for a bad email address.
Guilty :oops:

I noted this stupidity of mine whilst writing version 4 and I can assure that there are no occurences of "Exception" in v4.
The other was generated using the inherited class Swift_BadResponseException for an SMTP error.
But this is certainly correct. My Exceptions are hierarchical. If it comes from the Connection it should be a ConnectionException, but it may have a more specific type. Catching a ConnectionException will still catch a BadResponseException.
In your send function in Swift.php you create two different kinds of exceptions, both using the same PHP base class Exception. One for a bad recipient address and another for a bad sender address, neither with a code attached, so without hacking your code, it is impossible to tell them apart unless you compare error strings, which is very inefficient and a poor coding practice.
Haven't checked, but probably guilty :oops: Again, this sort of slap-happy code does not exist in v4.
Unless you are doing something unique when you extend the PHP base class Exception, I think it's inefficient to create a different inherited Exception class for each type of exception, which does not alter in anyway the way the base class works, in lieu of the simpler method of passing a unique code per unique exception to the base Exception class. That's the reason the base class accepts both an error message and a code.

You can define all the SWIFT_EXCEPTION_CODES in a separate file, so that anyone using your library can change them to whatever they like to avoid code conflicts with another library, if that should arise.
Ok, I'll consider using Exception codes in v4, but I won't stop subclassing my Exceptions I'm afraid. They increase in specificity for a reason.

Re: Exception handling

Posted: Thu Feb 05, 2009 6:00 am
by rrcatto
Chris Corbyn wrote:Ok, I'll consider using Exception codes in v4, but I won't stop subclassing my Exceptions I'm afraid. They increase in specificity for a reason.
I think some coders will prefer your style of catching exceptions.

I prefer catching them on codes. If you number them using a binary hierarchy, you can use bitwise operators to achieve the desired granularity.

I need to know exactly what caused the exception in some cases, so that I can take a specific action, but in other cases, a generic solution could be applied to general failures.

For example, if there is an authentication problem with smtp, I may want to try a different transport, or just terminate and ask the user to investigate the reason for the authentication failure. If the problem is that the smtp server cannot accept more connections, then I'd prefer to sleep for X number of seconds before continuing.

Re: Exception handling

Posted: Thu Feb 05, 2009 6:15 am
by Chris Corbyn
rrcatto wrote:
Chris Corbyn wrote:Ok, I'll consider using Exception codes in v4, but I won't stop subclassing my Exceptions I'm afraid. They increase in specificity for a reason.
I think some coders will prefer your style of catching exceptions.

I prefer catching them on codes. If you number them using a binary hierarchy, you can use bitwise operators to achieve the desired granularity.

I need to know exactly what caused the exception in some cases, so that I can take a specific action, but in other cases, a generic solution could be applied to general failures.

For example, if there is an authentication problem with smtp, I may want to try a different transport, or just terminate and ask the user to investigate the reason for the authentication failure. If the problem is that the smtp server cannot accept more connections, then I'd prefer to sleep for X number of seconds before continuing.
See, my way of thinking is that if I want people to detect authentication errors differently to general Connection errors, but still be connection errors I'll create a subclass of ConnectionException, whereas what you're saying is that you'd have a code that is "$connectionError | $someValue" instead. This feels a little hacky and hard to follow to me. It's certainly a good thing to do for non-exception "errors" just like PHP itself throws, but for Exceptions you're better off making use of the Exception types.

I didn't forget to add fine-grained error codes, I just didn't think I needed that level of granularity, and if I do need it I'll use new Exception types :) I've actually never really seen people using error codes with Exceptions much, but maybe that's just me.

I can certainly add error codes to the Exceptions I throw though if it's actually going to be useful. I've never heard of anybody doing it that way you do though in all my years of programming.

Re: Exception handling

Posted: Thu Feb 05, 2009 7:36 am
by rrcatto
Chris Corbyn wrote:See, my way of thinking is that if I want people to detect authentication errors differently to general Connection errors, but still be connection errors I'll create a subclass of ConnectionException, whereas what you're saying is that you'd have a code that is "$connectionError | $someValue" instead.
I'll use hex error codes to illustrate:

0x1000 - for exceptions related to message format
0x1100 - for exceptions related to bad email addresses
0x1110 - for a bad sender address
0x1120 - for a bad from address

0x2000 - for connection exceptions
0x2100 - for exceptions with smtp transport
0x2200 - for exceptions with sendmail transport
0x2110 - for exceptions with smtp authentication

You have your hierarchy, plus specifics so you can drill down to exactly what went wrong while preserving your broad groupings of exceptions. If 4 digits are not enough to cover all your exception categories and sub-categories, just use a longer sequence.

If there was a conflict in codespace with another php library, one can simply add an extra digit on after the 'x' to resolve the conflict. Code using your library would not need to change, because you simply use defines and OR them with a mask to get the error you want to handle.

e.g.

Code: Select all

$ecode = $e->getcode()
if ($ecode | SWIFT_EXCEPTION_MASK == SWIFT_EXCEPTION_SMTP_AUTHENTICATION_FAILURE) {
  // handle smtp authentication failure
}
There is definitely call for exception codes. I've seen others using them.

I'd rather catch a general PHP base class Exception and use the code to decide how to handle it.

Re: Exception handling

Posted: Thu Feb 05, 2009 7:53 am
by Chris Corbyn
My classes are designed in a way that they do not know about each other, so they should not know what codes are safe to use and what are not. I mean, SendmailTransport does not know that SmtpTransport exists, and it certainly does not know about its internals, nor the other way around.

I honestly don't know of any other PHP libraries that offer such fine-grained exceptions, nor can I see an overwhelming practical use for it in Swift Mailer. It's a fairly huge amount of work to make all of my exceptions include a very specific error code that does not clash as time goes on. Can anyone else chime into this thread to confirm if this is needed or not?

I've never been much of a fan of cryptic magic numbers, I'd still prefer the subclass route.

Re: Exception handling

Posted: Thu Feb 05, 2009 9:00 am
by rrcatto
Chris Corbyn wrote:My classes are designed in a way that they do not know about each other, so they should not know what codes are safe to use and what are not. I mean, SendmailTransport does not know that SmtpTransport exists, and it certainly does not know about its internals, nor the other way around.

I honestly don't know of any other PHP libraries that offer such fine-grained exceptions, nor can I see an overwhelming practical use for it in Swift Mailer. It's a fairly huge amount of work to make all of my exceptions include a very specific error code that does not clash as time goes on. Can anyone else chime into this thread to confirm if this is needed or not?

I've never been much of a fan of cryptic magic numbers, I'd still prefer the subclass route.
Actually, I've realised that codespace conflict with other PHP libraries is not an issue if I catch a Swift_Exception rather than the PHP general base class Exception.

Swift exception codes should be unique across the entire library, though, and that would not be difficult to maintain.

Can you tell me in which files you throw exceptions?

I'm willing to help code this if you would like to add this feature.

Re: Exception handling

Posted: Thu Feb 05, 2009 10:14 am
by xdecock
following grep (it must be able to catch almost everything, however, i'll not promise it)

Code: Select all

 
xavier@desktop-xavier:~/swift/swiftmailer/lib/classes$ grep -Ri 'throw ' *
Swift/Plugins/LoggerPlugin.php:    throw new Swift_Transport_TransportException($message);
Swift/DependencyContainer.php:      throw new Swift_DependencyException(
Swift/DependencyContainer.php:      throw new BadMethodCallException(
Swift/KeyCache/ArrayKeyCache.php:        throw new Swift_SwiftException(
Swift/KeyCache/ArrayKeyCache.php:        throw new Swift_SwiftException(
Swift/KeyCache/DiskKeyCache.php:        throw new Swift_SwiftException(
Swift/KeyCache/DiskKeyCache.php:        throw new Swift_SwiftException(
Swift/KeyCache/DiskKeyCache.php:        throw new Swift_IoException('Failed to create cache directory ' . $cacheDir);
Swift/Mime/SimpleMimeEntity.php:      throw new Exception('Mime boundary set is not RFC 2046 compliant.');
Swift/Mime/Headers/IdentificationHeader.php:        throw new Swift_RfcComplianceException(
Swift/Mime/Headers/PathHeader.php:      throw new Swift_RfcComplianceException(
Swift/Mime/Headers/AbstractHeader.php:      throw new Swift_RfcComplianceException(
Swift/Mime/Headers/MailboxHeader.php:      throw new Swift_RfcComplianceException(
Swift/Transport/LoadBalancedTransport.php:      throw new Swift_Transport_TransportException(
Swift/Transport/AbstractSmtpTransport.php:      throw new Swift_Transport_TransportException(
Swift/Transport/AbstractSmtpTransport.php:      throw $e;
Swift/Transport/AbstractSmtpTransport.php:  /** Throw a TransportException, first sending it to any listeners */
Swift/Transport/AbstractSmtpTransport.php:        throw $e;
Swift/Transport/AbstractSmtpTransport.php:      throw $e;
Swift/Transport/FailoverTransport.php:      throw new Swift_Transport_TransportException(
Swift/Transport/Esmtp/AuthHandler.php:      throw new Swift_Transport_TransportException(
Swift/Transport/StreamBuffer.php:      throw new Swift_Transport_TransportException(
Swift/Transport/StreamBuffer.php:      throw new Swift_Transport_TransportException(
Swift/ByteStream/FileByteStream.php:        throw new Swift_IoException(
Swift/ByteStream/FileByteStream.php:        throw new Swift_IoException(
xavier@desktop-xavier:~/swift/swiftmailer/lib/classes$ 
 

Re: Exception handling

Posted: Thu Feb 05, 2009 10:30 am
by xdecock
however, my problems comes from "swift_plugins_exceptions"

Let's imagine:

SwiftMailer has some success.

Some devs provides plugins not in mainstream for various reasons.

So we Have: Swift_Plugins_PluginA throwing Swift_Exception with Error Code 0xAB8001 (0xABXXXX for plugins, 0xAB[b1yyy]XXX for external plugins)

why Swift_Plugins_PluginB would use another code than 0xAB80001 for his own exception?

furthermore, it requires constants definitions in a separate file (in order to provide easy upgrade of "swift Core")

For those reasons, i find "globally registered" Exception Codes dangerous.

An Swift_ConnectionException_UnexpectedResponse would use numeric code best by providing the response code than using this complex system of typing Exceptions via Codes...

The only usefullness i find for this is something like this:

Code: Select all

 
try {
 
} catch (Exception $e){
  switch($e->getCode()){
    case SWIFT_EXCEPTION_BLABLA:
       // do stuff
    break;
    case SWIFT_EXCEPTION_GLUP:
        // do something else
    break;
    default:
        // Log & drop
    break;
  }
}
 
This leads to a complex switch where you can forget an break; leading to code Exception triggering more code than expected.

a "advantage" of this is the possibility of cascading treatment,

however, i'll recommand catching specific exceptions first and after this, more generic ones & so on

Code: Select all

 
try {
  // Do dangerous stuffs
} catch (Exception_A_A_A_B $e) {
  //do think specifics to A_A_A_B & childs here
} catch (Exception_A_A_A_C $e) {
  // Do think specifics to A_A_A_C & childs
} catch (Exception_A_A_A $e) {
  // Do think specifics to A_A_A & childs except A_A_A_C & A_A_A_B
} catch (Exception $e){
  // Default action
}
 

Re: Exception handling

Posted: Thu Feb 05, 2009 11:12 am
by rrcatto
xdecock wrote:following grep (it must be able to catch almost everything, however, i'll not promise it)
Looking at the names of those exceptions classes, I'd like to suggest a standardised naming convention be applied to class names of derived Swift_Exception classes.

The parent Swift exception class that extends the PHP general base class Exception should be called Swift_Exception, which I think it is.

Child classes should be named Swift_Exception_ChildName.

That will make it easy to follow it up the chain.

Re: Exception handling

Posted: Thu Feb 05, 2009 1:51 pm
by John Cartwright
rrcatto wrote:
xdecock wrote:following grep (it must be able to catch almost everything, however, i'll not promise it)
Looking at the names of those exceptions classes, I'd like to suggest a standardised naming convention be applied to class names of derived Swift_Exception classes.

The parent Swift exception class that extends the PHP general base class Exception should be called Swift_Exception, which I think it is.

Child classes should be named Swift_Exception_ChildName.

That will make it easy to follow it up the chain.
I've actually never even considered doing it this way. However, I would vote against doing so, considering the exceptions are a subset of the component, and are not components themselves.