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.

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

feat: better exception handling #40

Merged
merged 18 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,13 @@ will be thrown, and any error(s) returned by the SmarterU API will be listed in
the exception message. A list of all possible SmarterU errors can be found
[here](https://support.smarteru.com/docs/api-error-codes).

## Logging

This library uses the `psr/log` library to log information about failed requests
to a logger of your choosing. If an instance of `Psr\Log\LoggerInterface` is
inject using `Client::setLogger()` then any failed requests will be logged and
a sanitized version of the request and response will be sent to the logger.

## Contributing

If you would like to make a contribution to this library, you may do so using
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
],
"require": {
"guzzlehttp/guzzle": "^7.0",
"ext-SimpleXML": "*"
"ext-SimpleXML": "*",
"psr/log": "^3.0"
},
"require-dev": {
"phpunit/phpunit": "9.5",
Expand Down
71 changes: 70 additions & 1 deletion src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,22 @@
use DateTime;
use GuzzleHttp\Client as HttpClient;
use GuzzleHttp\Exception\ClientException;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use SimpleXMLElement;

/**
* The Client class makes API calls and translates the response to the
* appropriate object.
*/
class Client {
#region traits

use LoggerAwareTrait;

#endregion traits

#region constants

/**
Expand Down Expand Up @@ -107,6 +116,7 @@ class Client {
*/
protected XMLGenerator $xmlGenerator;


#endregion properties

/**
Expand All @@ -124,13 +134,16 @@ class Client {
* You must set the user API key via the constructor or
* `setUserApi` before invoking methods which interact with the
* SmarterU API
* @param LoggerInterface|null $logger The logger used to record API errors.
*/
public function __construct(
string $apiKey,
string $apiUserKey
string $apiUserKey,
?LoggerInterface $logger = null
) {
$this->setAccountApi($apiKey);
$this->setUserApi($apiUserKey);
$this->setLogger($logger ?? new NullLogger());
}

/**
Expand Down Expand Up @@ -254,6 +267,7 @@ public function createUser(User $user): User {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -355,6 +369,7 @@ public function listUsers(ListUsersQuery $query): array {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -444,6 +459,7 @@ public function updateUser(User $user): User {

if ((string) $bodyAsXml->Result === 'Failed') {
$errors = $this->getErrorCodesFromXmlElement($bodyAsXml->Errors);
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$errors
Expand Down Expand Up @@ -546,6 +562,7 @@ public function createGroup(Group $group): Group {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -631,6 +648,7 @@ public function listGroups(ListGroupsQuery $query): array

if ((string) $bodyAsXml->Result === 'Failed') {
$errors = $this->getErrorCodesFromXmlElement($bodyAsXml->Errors);
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$errors
Expand Down Expand Up @@ -696,6 +714,7 @@ public function updateGroup(Group $group): Group {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -752,6 +771,7 @@ public function addUsersToGroup(array $users, Group $group): Group {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -804,6 +824,7 @@ public function removeUsersFromGroup(array $users, Group $group): Group {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -877,6 +898,7 @@ public function grantPermissions(
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -950,6 +972,7 @@ public function revokePermissions(
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -997,6 +1020,7 @@ public function getLearnerReport(GetLearnerReportQuery $query): array {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -1200,6 +1224,7 @@ protected function getUser(GetUserQuery $query): ?User {
}
}

$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$errors
Expand Down Expand Up @@ -1305,6 +1330,7 @@ protected function getUserGroups(GetUserQuery $query): array {
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -1367,6 +1393,7 @@ protected function getGroup(GetGroupQuery $query): ?Group {
}
}

$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$errors
Expand Down Expand Up @@ -1447,6 +1474,7 @@ protected function requestExternalAuthorization(
$bodyAsXml = simplexml_load_string((string) $response->getBody());

if ((string) $bodyAsXml->Result === 'Failed') {
$this->logFailedRequest($xml, (string) $response->getBody());
throw new SmarterUException(
self::SMARTERU_EXCEPTION_MESSAGE,
$this->getErrorCodesFromXmlElement($bodyAsXml->Errors)
Expand Down Expand Up @@ -1476,4 +1504,45 @@ private function getErrorCodesFromXmlElement(SimpleXMLElement $errors): array {
}
return $errorCodes;
}

/**
* Accepts a request XML string and sanitizes it for logging purposes.
*
* @param string $request The XML request to sanitize.
* @return string The sanitized XML request.
*/
public function sanitizeRequestXML(string $request): string {
// Scrub AccountAPI key so we don't expose a secret in logs.
$sanitizedRequest = preg_replace(
'/<AccountAPI>.*<\/AccountAPI>/',
'<AccountAPI>********</AccountAPI>',
$request
);

// Scrub UserAPI key so we don't expose a secret in logs.
$sanitizedRequest = preg_replace(
'/<UserAPI>.*<\/UserAPI>/',
'<UserAPI>********</UserAPI>',
$sanitizedRequest
);

return $sanitizedRequest;
}

/**
* Logs a failed request.
*
* @param string $request The XML request that failed.
* @param string $response The XML response that was received.
*/
private function logFailedRequest(string $request, string $response) {
// Log the request and response.
$this->logger->error(
'Failed to make request to SmarterU API. See context for request/response details.',
[
'request' => $this->sanitizeRequestXML($request),
'response' => $response
]
);
}
}
12 changes: 11 additions & 1 deletion tests/Client/AddUsersToGroupClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,17 @@ public function testAddUsersToGroupThrowsExceptionWhenFatalErrorReturned() {
$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);
$httpClient = new HttpClient(['handler' => $handlerStack]);
$client->setHttpClient($httpClient);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU><AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>updateGroup</Method><Parameters><Group><Identifier><Name>My Group</Name></Identifier><Users><User><Email>[email protected]</Email><UserAction>Add</UserAction><HomeGroup>0</HomeGroup><Permissions/></User></Users><LearningModules/><SubscriptionVariants/></Group></Parameters></SmarterU>\n",
'response' => $body
])
);
$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
12 changes: 11 additions & 1 deletion tests/Client/CreateGroupClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,18 @@ public function testCreateGroupThrowsExceptionWhenFatalErrorReturned() {
$handlerStack->push($history);

$httpClient = new HttpClient(['handler' => $handlerStack]);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU><AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>createGroup</Method><Parameters><Group><Name>My Group</Name><GroupID>12</GroupID><Status>Active</Status><Description>This is a group created for testing.</Description><HomeGroupMessage>Home Group</HomeGroupMessage><NotificationEmails><NotificationEmail>[email protected]</NotificationEmail><NotificationEmail>[email protected]</NotificationEmail></NotificationEmails><UserHelpOverrideDefault>0</UserHelpOverrideDefault><UserHelpEnabled>1</UserHelpEnabled><UserHelpEmail>[email protected],[email protected]</UserHelpEmail><UserHelpText>Help Message</UserHelpText><Tags2><Tag2><TagID>1</TagID><TagValues>Tag1 values</TagValues></Tag2><Tag2><TagID>2</TagID><TagValues>Tag2 values</TagValues></Tag2></Tags2><UserLimit><Enabled>1</Enabled><Amount>50</Amount></UserLimit><Users/><LearningModules><LearningModule><ID>4</ID><AllowSelfEnroll>1</AllowSelfEnroll><AutoEnroll>0</AutoEnroll></LearningModule><LearningModule><ID>5</ID><AllowSelfEnroll>0</AllowSelfEnroll><AutoEnroll>1</AutoEnroll></LearningModule></LearningModules><SubscriptionVariants><SubscriptionVariant><ID>6</ID><RequiresCredits>1</RequiresCredits></SubscriptionVariant><SubscriptionVariant><ID>7</ID><RequiresCredits>0</RequiresCredits></SubscriptionVariant></SubscriptionVariants><DashboardSetID>8</DashboardSetID></Group></Parameters></SmarterU>\n",
'response' => $body
])
);

$client->setHttpClient($httpClient);
$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
12 changes: 11 additions & 1 deletion tests/Client/CreateUserClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,18 @@ public function testCreateUserThrowsExceptionWhenFatalErrorReturned() {
$handlerStack->push($history);

$httpClient = new HttpClient(['handler' => $handlerStack]);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU><AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>createUser</Method><Parameters><User><Info><Email>[email protected]</Email><EmployeeID>1</EmployeeID><GivenName>PHP</GivenName><Surname>Unit</Surname><Password>password</Password><Timezone>EST</Timezone><LearnerNotifications>1</LearnerNotifications><SupervisorNotifications>1</SupervisorNotifications><SendEmailTo>Self</SendEmailTo><AlternateEmail>[email protected]</AlternateEmail><AuthenticationType>External</AuthenticationType></Info><Profile><Supervisors><Supervisor>supervisor1</Supervisor><Supervisor>supervisor2</Supervisor></Supervisors><Organization>organization</Organization><Teams><Team>team1</Team><Team>team2</Team></Teams><Language>English</Language><Status>Active</Status><Title>Title</Title><Division>division</Division><AllowFeedback>1</AllowFeedback><PhonePrimary>555-555-5555</PhonePrimary><PhoneAlternate>555-555-1234</PhoneAlternate><PhoneMobile>555-555-4321</PhoneMobile><Fax>555-555-5432</Fax><Website>https://localhost</Website><Address1>123 Main St</Address1><Address2>Apt. 1</Address2><City>Anytown</City><Province>Pennsylvania</Province><Country>United States</Country><PostalCode>12345</PostalCode><SendMailTo>Personal</SendMailTo><ReceiveNotifications>1</ReceiveNotifications><HomeGroup>My Home Group</HomeGroup></Profile><Groups><Group><GroupName>My Home Group</GroupName><GroupPermissions/></Group></Groups><Venues/><Wages/></User></Parameters></SmarterU>\n",
'response' => $body
])
);

$client->setHttpClient($httpClient);
$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
12 changes: 11 additions & 1 deletion tests/Client/GetGroupClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,18 @@ public function testGetGroupThrowsExceptionWhenFatalErrorReturned() {
$handlerStack->push($history);

$httpClient = new HttpClient(['handler' => $handlerStack]);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU>\n<AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>getGroup</Method><Parameters><Group><Name>My Group</Name></Group></Parameters></SmarterU>\n",
'response' => $body
])
);

$client->setHttpClient($httpClient);
$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
13 changes: 12 additions & 1 deletion tests/Client/GetLearnerReportClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,18 @@ public function testAddUsersToGroupThrowsExceptionWhenFatalErrorReturned() {
$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);
$httpClient = new HttpClient(['handler' => $handlerStack]);
$client->setHttpClient($httpClient);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU><AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>getLearnerReport</Method><Parameters><Report><Page>1</Page><PageSize>50</PageSize><Filters><EnrollmentID>1</EnrollmentID><Groups><GroupStatus>Active</GroupStatus></Groups><Enrollments/><Users><UserStatus>Active</UserStatus></Users></Filters></Report><Columns/><CustomFields/></Parameters></SmarterU>\n",
'response' => $body
])
);

$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
14 changes: 12 additions & 2 deletions tests/Client/GetUserClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,18 @@ public function testGetUserThrowsExceptionWhenFatalErrorReturned() {
$handlerStack->push($history);

$httpClient = new HttpClient(['handler' => $handlerStack]);

$client->setHttpClient($httpClient);
$logger = $this->createMock(\Psr\Log\LoggerInterface::class);
$logger->expects($this->once())->method('error')->with(
$this->identicalTo('Failed to make request to SmarterU API. See context for request/response details.'),
$this->identicalTo([
'request' => "<?xml version=\"1.0\"?>\n<SmarterU>\n<AccountAPI>********</AccountAPI><UserAPI>********</UserAPI><Method>getUser</Method><Parameters><User><ID>1</ID></User></Parameters></SmarterU>\n",
'response' => $body
])
);

$client
->setHttpClient($httpClient)
->setLogger($logger);

// Make the request. Because we want to inspect custom exception
// properties we'll handle the try/catch/cache of the exception
Expand Down
Loading
Loading