Add a more helpful error message when the transport is undefined.

When creating a profile with no transport, or just not setting a transport fatal errors were emitted. A proper exception seems nicer. Refs #3982
cakephp · Jul 16, 2014 · 476cae0 · 476cae0
1 parent 4733a8e
commit 476cae0
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 19 deletions.
diff --git a/src/Network/Email/Email.php b/src/Network/Email/Email.php
@@ -1224,25 +1224,42 @@ public function send($content = null) {
 
 		$this->_message = $this->_render($this->_wrap($content));
 
-		$contents = $this->transport()->send($this);
-		if (!empty($this->_profile['log'])) {
-			$config = [
-				'level' => LOG_DEBUG,
-				'scope' => 'email'
-			];
-			if ($this->_profile['log'] !== true) {
-				if (!is_array($this->_profile['log'])) {
-					$this->_profile['log'] = ['level' => $this->_profile['log']];
-				}
-				$config = $this->_profile['log'] + $config;
+		$transport = $this->transport();
+		if (!$transport) {
+			$msg = 'Cannot send email, transport was not defined. Did you call transport() or define ' .
+				 ' a transport in the set profile?';
+			throw new Error\SocketException($msg);
+		}
+		$contents = $transport->send($this);
+		$this->_logDelivery($contents);
+		return $contents;
+	}
+
+/**
+ * Log the email message delivery.
+ *
+ * @param array $contents The content with 'headers' and 'message' keys.
+ * @return void
+ */
+	protected function _logDelivery($contents) {
+		if (empty($this->_profile['log'])) {
+			return;
+		}
+		$config = [
+			'level' => LOG_DEBUG,
+			'scope' => 'email'
+		];
+		if ($this->_profile['log'] !== true) {
+			if (!is_array($this->_profile['log'])) {
+				$this->_profile['log'] = ['level' => $this->_profile['log']];
 			}
-			Log::write(
-				$config['level'],
-				PHP_EOL . $contents['headers'] . PHP_EOL . $contents['message'],
-				$config['scope']
-			);
+			$config = $this->_profile['log'] + $config;
 		}
-		return $contents;
+		Log::write(
+			$config['level'],
+			PHP_EOL . $contents['headers'] . PHP_EOL . $contents['message'],
+			$config['scope']
+		);
 	}
 
 /**

diff --git a/tests/TestCase/Network/Email/EmailTest.php b/tests/TestCase/Network/Email/EmailTest.php
@@ -1019,28 +1019,42 @@ public function testSendWithContent() {
 /**
  * testSendWithoutFrom method
  *
+ * @expectedException Cake\Network\Error\SocketException
  * @return void
  */
 	public function testSendWithoutFrom() {
 		$this->CakeEmail->transport('debug');
 		$this->CakeEmail->to('cake@cakephp.org');
 		$this->CakeEmail->subject('My title');
 		$this->CakeEmail->profile(array('empty'));
-		$this->setExpectedException('Cake\Network\Error\SocketException');
 		$this->CakeEmail->send("Forgot to set From");
 	}
 
 /**
  * testSendWithoutTo method
  *
+ * @expectedException Cake\Network\Error\SocketException
  * @return void
  */
 	public function testSendWithoutTo() {
 		$this->CakeEmail->transport('debug');
 		$this->CakeEmail->from('cake@cakephp.org');
 		$this->CakeEmail->subject('My title');
 		$this->CakeEmail->profile(array('empty'));
-		$this->setExpectedException('Cake\Network\Error\SocketException');
+		$this->CakeEmail->send("Forgot to set To");
+	}
+
+/**
+ * test send without a transport method
+ *
+ * @expectedException Cake\Network\Error\SocketException
+ * @expectedExceptionMessage Cannot send email, transport was not defined.
+ * @return void
+ */
+	public function testSendWithoutTransport() {
+		$this->CakeEmail->to('cake@cakephp.org');
+		$this->CakeEmail->from('cake@cakephp.org');
+		$this->CakeEmail->subject('My title');
 		$this->CakeEmail->send("Forgot to set To");
 	}