MA-1084 #time 1h Implemented better exception handling and added unit

tests for said exception handling

Author: beardyman

README.md

@@ -87,11 +87,12 @@ try {
 ## Development
 
 ### Setup
-
 Run `composer install` inside the directory to install dependecies and development tools.
 
 ### Testing
-Once all the dependencies are installed, you can execute the unit tests using `vendor/bin/phpunit --bootstrap test/unit/bootstrap.php ./test/unit`
+Once all the dependencies are installed, you can execute the unit tests using `vendor/bin/phpunit --bootstrap test/unit/bootstrap.php ./test/unit`.
+
+If you're interested in code coverage, you can add the `--coverage` flag for phpunit like so: ```phpunit --coverage-html test/output/report --bootstrap test/unit/bootstrap.php ./test/unit```
 
 ### Contributing
 Guidelines for adding issues

examples/transmission/configuration_based.php renamed to examples/transmission/simple_send.php

@@ -1,7 +1,7 @@
 <?php
 namespace SparkPost;
 use Guzzle\Http\Client;
-use Guzzle\Http\Exception\RequestException;
+use Guzzle\Http\Exception\ClientErrorResponseException;
 
 /**
  * @desc SDK interface for managing transmissions
@@ -98,18 +98,6 @@ private static function getBaseUrl($config) {
 		return $config['protocol'] . '://' . $config['host'] . ($config['port'] ? ':' . $config['port'] : '') .  $baseUrl;
 	}
 
-	/**
-	 * Helper function for extracting the response status code out of request thrown exceptions
-	 * @param string $exceptionMessage
-	 * @return number
-	 */
-	private static function getStatusCode ($exceptionMessage) {
-		$messageParts = explode("\n", $exceptionMessage);
-		$codeLine = explode('] ', $messageParts[1]);
-		$code = trim($codeLine[1]);
-		return (int)$code;
-	}
-	
 	/**
 	 * @desc Method for issuing POST request to the Transmissions API
 	 *
@@ -150,11 +138,19 @@ public static function send($transmissionConfig) {
 		try {
 			$response = $request->post(self::getBaseUrl($hostConfig), array('authorization' => $hostConfig['key']), json_encode($model), array("verify"=>$hostConfig['strictSSL']))->send();
 			return $response->json();
-		} catch (RequestException $exception) {
+		} 
+		/*
+		 * Handles 4XX responses
+		 */
+		catch (ClientErrorResponseException $exception) {
 			$response = $exception->getResponse();
 			$responseArray = $response->json();
-			throw new \Exception(json_encode($responseArray['errors']));
-		} catch (\Exception $exception) {
+			throw new \Exception(json_encode($responseArray['errors']));	
+		} 
+		/*
+		 * Handles 5XX Errors, Configuration Errors, and a catch all for other errors
+		 */
+		catch (\Exception $exception) { 
 			throw new \Exception('Unable to contact Transmissions API: '. $exception->getMessage());
 		}
 	}
@@ -184,15 +180,23 @@ private static function fetch ($transmissionID = null) {
 		try {	
 			$response = $request->get($url, array('authorization' => $hostConfig['key']), array("verify"=>$hostConfig['strictSSL']))->send();
 			return $response->json();
-		} catch (\Exception $exception) {
-			$statusCode = self::getStatusCode($exception->getMessage());
+		} 
+		/*
+		 * Handles 4XX responses
+		 */
+		catch (ClientErrorResponseException $exception) {
+			$response = $exception->getResponse();
+			$statusCode = $response->getStatusCode();
 			if($statusCode === 404) {
 				throw new \Exception("The specified Transmission ID does not exist", 404);
-			}else if ($statusCode !== null){
-				throw new \Exception("Received bad response from Transmission API: ". $statusCode);
-			} else {
-				throw new \Exception('Unable to contact Transmissions API: '. $exception->getMessage());
 			}
+			throw new \Exception("Received bad response from Transmission API: ". $statusCode);
+		}
+		/*
+		 * Handles 5XX Errors, Configuration Errors, and a catch all for other errors
+		 */
+		catch (\Exception $exception) {
+			throw new \Exception('Unable to contact Transmissions API: '. $exception->getMessage());
 		}
 	}
 

lib/SparkPost/Transmission.php

@@ -96,11 +96,13 @@ public function testSetHtml() {
 		$this->assertEquals($value, $this->email->model['html']);
 	}
 
+	/**
+	 * @desc test that adding a category throws an exception since we don't support tags at transmission level yet
+	 * @expectedException Exception
+	 * @expectedExceptionMessage Adding categories is not yet supported
+	 */
 	public function testAddCategory() {
-		$value = 'Category A';
 		$this->email->addCategory($value);
-	
-		$this->assertEquals(array($value), $this->email->model['tags']);
 	}
 
 	/**

test/unit/SendGridCompatibiility/EmailTest.php

@@ -89,6 +89,18 @@ public function testFindWithOtherBadResponse() {
 		Transmission::find('someId');
 	}
 
+	/**
+	 * @desc tests bad response
+	 * @expectedException Exception
+	 * @expectedExceptionMessageRegExp /Unable to contact Transmissions API:.* /
+	 */
+	public function testFindForCatchAllException() {
+		$mock = new MockPlugin();
+		$mock->addResponse(new Response(500));
+		$this->client->addSubscriber($mock);
+		Transmission::find('someId');
+	}
+	
 	/**
 	 * @desc tests happy path
 	 */
@@ -107,13 +119,26 @@ public function testSuccessfulSend() {
 	 * @expectedException Exception
 	 * @expectedExceptionMessage ["This is a fake error"]
 	 */
-	public function testSendForRequestException() {
+	public function testSendFor400Exception() {
 		$body = array('errors'=>array('This is a fake error'));
 		$mock = new MockPlugin();
 		$mock->addResponse(new Response(400, array(), json_encode($body)));
 		$this->client->addSubscriber($mock);
 		Transmission::send(array('text'=>'awesome email'));
 	}
 
+	
+	/**
+	* @desc tests bad response
+	* @expectedException Exception
+	* @expectedExceptionMessageRegExp /Unable to contact Transmissions API:.* /
+	*/
+	public function testSendForCatchAllException() {
+		$mock = new MockPlugin();
+		$mock->addResponse(new Response(500));
+		$this->client->addSubscriber($mock);
+		Transmission::send(array('text'=>'awesome email'));
+	}
+	
 }
 ?>