Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mailer exception unification #13

Closed
wants to merge 5 commits into from
Closed

Mailer exception unification #13

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e241540
Mailer exception unification
SendiMyrkr Jun 25, 2015
90097d4
Repair coding style
SendiMyrkr Jun 25, 2015
de13c6d
Moving SmtpException
SendiMyrkr Jun 25, 2015
31695dc
Adding doc @throws to mailer interface
SendiMyrkr Jun 25, 2015
628a81b
Separate SendmailException and SendFailedException as interface
SendiMyrkr Jun 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Mail/IMailer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ interface IMailer
/**
* Sends email.
* @return void
*
* @throws Nette\Mail\SendFailedException
*/
function send(Message $mail);

Expand Down
2 changes: 1 addition & 1 deletion src/Mail/SendmailMailer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function send(Message $mail)
$info = ": $message";
});
if ($res === FALSE) {
throw new Nette\InvalidStateException("Unable to send email$info.");
throw new SendFailedException("Unable to send email$info.");
}
}

Expand Down
10 changes: 0 additions & 10 deletions src/Mail/SmtpMailer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,3 @@ protected function read()
}

}


/**
* SMTP mailer exception.
*
* @author David Grudl
*/
class SmtpException extends \Exception
{
}
30 changes: 30 additions & 0 deletions src/Mail/exceptions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/**
* This file is part of the Nette Framework (http://nette.org)
* Copyright (c) 2004 David Grudl (http://davidgrudl.com)
*/

namespace Nette\Mail;

use Nette;


/**
* Send failed exception.
*
* @author Jan Dvořák
*/
class SendFailedException extends Nette\InvalidStateException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tohle nemá dědit od InvalidStateException

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Já vím, ale když to neudělám bude tam BC break

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nebude class SmtpException extends \Exception

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jak jsem psal, imho bude lepsi kdyz bude dedit - pro lepsi BC, ted nekdo u sendmail maileru treba chyta InvalidStateException ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ajo, sendmail háže InvalidStateException, tak pak ok

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Připadá mi jako zbytečnost mít zvlášť exception pro každý mailer. Existuje pro to usecase?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Já o žádným nevím, dělám to hlavně proto aby stačilo zachytáva jenom jednu vyjímku, ale kvůli BC stejně musí ta SmtpException zůstat, ne?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To jo, ale stačilo by přidat obecnou SendFailedException a SmtpException od ní podědit.

{
}


/**
* SMTP mailer exception.
*
* @author David Grudl
*/
class SmtpException extends SendFailedException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dědíš to naopak, tohle je špatně, SmtpException je obecnější

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, když teď nad tím tak přemýšlím, tak už chápu proč se to snaží udělat takhle... protože SendFailedException může pak házet i ten sendmail...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ale pak to nemůžeš poslat v SendMailMaileru a tím to ztrácí smysl, myslel jsem, že bych v SmtpMaileru dal taky tu vyjímku SendFailed ale bude to BC break

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tak udělej interface, něco jako

class SmtpException extends \Exception {}
class SendmailException extends \Exception {}

interface SendFailedException {}
class SmtpSendFailedException extends \Exception implements SendFailedException {}
class SendmailSendFailedException extends \Exception implements SendFailedException {}

(jenom nástřel, je potřeba to domyslet)

{
}