Comments (13)
I'm actually not even sure if it's a good idea to have this parseResponse method at all. You should just return the response so the consuming entity can deal with it as it pleases. Or, if you really want to process the response for the user, you need to map the possible API error responses as listed here http://developer.postmarkapp.com/developer-api-overview.html#error-codes and return this information to the consuming entity.
from slmmail.
Hi,
The reason we did unify the exception is to make it possible to switch from one provider to another without changing code. This is because some providers like elasticemail have VERY POOR support for errors, so this way we are able to make everything works the same. So I think this is a good idea. You can catch for SlmMail exception interface, even without knowing which provider you are using internally.
However, I agree that any exception that we could set as previous exception should be done. Don't hesitate to do a PR if you feel this way, we will be glad to review.
Envoyé de mon iPhone
Le 6 nov. 2014 à 17:16, Markus [email protected] a écrit :
I'm actually not even sure if it's a good idea to have this parseResponse method at all. You should just return the response so the consuming entity can deal with it as it pleases. Or, if you really want to process the response for the user, you need to map the possible API error responses as listed here http://developer.postmarkapp.com/developer-api-overview.html#error-codes and return this information to the consuming entity.
—
Reply to this email directly or view it on GitHub.
from slmmail.
Another problem is that the parseResponse is private, which means I can't even override it to solve the problem in my Service extension. I don't feel like providing a pull request after what you've done with my last one. You're welcome for my efforts to point the problems out to you, though. Happy fixing.
from slmmail.
@markushausammann the API error codes postmark provides are only valid for 422 errors. The exception you highlight is the generic RuntimeException, but actually a ValidationErrorException is thrown with a specific message (postmark calls this not a "validation error" but "input error", the idea is the same). I am talking about this exception.
Would it be sufficient to set an exception code besides the message for you? Then you can catch the specific exception and grab the exception code. It will change SlmMail to this:
case 422:
$code = (int) $result['ErrorCode'];
$message = sprintf(
'An error occured on Postmark (error code %s), message: %s',
$result['ErrorCode'],
$result['Message']
);
throw new Exception\ValidationErrorException($message, $code);
Your userland code will become then:
use SlmMail\Service\Exception\ValidationErrorException;
try {
$transport->send($message);
} catch (ValidationErrorException $e) {
$code = $e->getCode();
// handle $code according to
// http://developer.postmarkapp.com/developer-api-overview.html#error-codes
}
Another problem is that the parseResponse is private, which means I can't even override it to solve the problem in my Service extension.
As with my above example, I don't think (yet) you need to override the method. If the above example is not enough for your use case, please elaborate so we can think along your lines to help.
from slmmail.
@juriansluiman yes, making the actual ErrorCode and Message extractable from the Exception would be the perfect solution, please change it in that way! This is really important because the reasons for the 422 response are very diverse and some of them can be handled gracefully and don't even need logging. For example 406 can happen and can easily be handled. 405 should probably alert somebody, 10 and 400 should log with high priority, etc.
from slmmail.
Closing, see #84
from slmmail.
Thank you!
from slmmail.
@juriansluiman would you mind tagging a minor release?
from slmmail.
Yes, but let's await merging first ;-)
from slmmail.
Sure, I thought you had that power :)
from slmmail.
Or is it running a build on travis or smth?
from slmmail.
@markushausammann all PRs are running against Travis and I prefer the policy not to merge PRs myself. So Michael reviews mines and vice versa (most of the times). Now all is green & merged, I tagged v1.5.1
from slmmail.
Many thanks!
from slmmail.
Related Issues (20)
- PHP Version HOT 1
- get was unable to fetch or create an instance for getServiceLocator HOT 1
- Mailgun as email transporter HOT 4
- Search API for Mandrill HOT 1
- ElasticEmail error when unauthorized
- laminas? HOT 5
- Transfer to JouwWeb organization
- Target PHP 7.2 and above
- Tag a release HOT 6
- Document how to use with Mezzio HOT 1
- Release 3.0
- Companies using SlmMail HOT 4
- Support new Postmark API requirements HOT 1
- SES - Enhancement request: switch to latest Aws ZF2 module HOT 11
- Send not return ID of message HOT 3
- Elastic Email error method HOT 2
- Unable to enable crypto on TCP connection api.mailgun.net HOT 4
- ElasticMail: enable custom headers HOT 1
- Zend 2 app + SendGrid emails sent are blocked by Gmail HOT 4
- Mailgun: support templates HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from slmmail.