Skip to content

Commit

Permalink
Have a global Exception catch in the webhook response methods to bett…
Browse files Browse the repository at this point in the history 
…er catch and isolate errors there, set a 500 response code on all errors
  • Loading branch information
Michael Babker committed Oct 12, 2016
1 parent f2b0bce commit 7708880
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 103 deletions.
94 changes: 56 additions & 38 deletions src/App/Tracker/Controller/Hooks/ReceiveCommentsHook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,39 +44,57 @@ protected function prepareResponse()

try
{
// Check to see if the comment is already in the database
$commentId = $this->db->setQuery(
$this->db->getQuery(true)
->select($this->db->quoteName('activities_id'))
->from($this->db->quoteName('#__activities'))
->where($this->db->quoteName('gh_comment_id') . ' = ' . (int) $this->hookData->comment->id)
)->loadResult();
}
catch (\RuntimeException $e)
{
$this->logger->error(
'Error checking the database for comment',
['comment_id' => (int) $this->hookData->comment->id, 'exception' => $e]
);
$this->getContainer()->get('app')->close();
}
try
{
// Check to see if the comment is already in the database
$commentId = $this->db->setQuery(
$this->db->getQuery(true)
->select($this->db->quoteName('activities_id'))
->from($this->db->quoteName('#__activities'))
->where($this->db->quoteName('gh_comment_id') . ' = ' . (int) $this->hookData->comment->id)
)->loadResult();
}
catch (\RuntimeException $e)
{
$logMessage = 'Error checking the database for comment';

// If the item is already in the database, update it; else, insert it
if ($commentId)
{
$result = $this->updateComment($commentId);
}
else
{
$result = $this->insertComment();
}
$this->logger->error(
$logMessage,
['comment_id' => (int) $this->hookData->comment->id, 'exception' => $e]
);
$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->response->message = 'Hook data processed unsuccessfully.';

if ($result)
{
$this->response->message = 'Hook data processed successfully.';
return;
}

// If the item is already in the database, update it; else, insert it
if ($commentId)
{
$result = $this->updateComment($commentId);
}
else
{
$result = $this->insertComment();
}

if ($result)
{
$this->response->message = 'Hook data processed successfully.';
}
else
{
$this->response->message = 'Hook data processed unsuccessfully.';
}
}
else
catch (\Exception $e)
{
$logMessage = 'Uncaught Exception processing comment webhook';

$this->logger->critical($logMessage, ['exception' => $e]);
$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->response->message = 'Hook data processed unsuccessfully.';
}
}
Expand Down Expand Up @@ -124,7 +142,7 @@ protected function insertComment()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -158,7 +176,7 @@ protected function insertComment()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -184,7 +202,7 @@ protected function insertComment()
'Error processing `onCommentAfterCreate` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -264,7 +282,7 @@ protected function insertIssue()
$this->project->gh_project,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -285,7 +303,7 @@ protected function insertIssue()
'Error processing `onCommentAfterCreateIssue` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -315,7 +333,7 @@ protected function insertIssue()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -364,7 +382,7 @@ protected function updateComment($id)
$this->project->gh_project,
$id
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -410,7 +428,7 @@ protected function updateComment($id)
$this->project->gh_project,
$id
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -434,7 +452,7 @@ protected function updateComment($id)
'Error processing `onCommentAfterUpdate` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down
82 changes: 46 additions & 36 deletions src/App/Tracker/Controller/Hooks/ReceiveIssuesHook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,39 @@ protected function prepareResponse()
if (!isset($this->hookData->issue->number) || !is_object($this->hookData))
{
// If we can't get the issue number exit.
$this->response->message = 'Hook data did not exists';
$this->response->message = 'Hook data does not exist';

return;
}

// If the item is already in the database, update it; else, insert it.
if ($this->checkIssueExists((int) $this->hookData->issue->number))
{
$result = $this->updateData();
}
else
try
{
$result = $this->insertData();
}
// If the item is already in the database, update it; else, insert it.
if ($this->checkIssueExists((int) $this->hookData->issue->number))
{
$result = $this->updateData();
}
else
{
$result = $this->insertData();
}

if ($result)
{
$this->response->message = 'Hook data processed successfully.';
if ($result)
{
$this->response->message = 'Hook data processed successfully.';
}
else
{
$this->response->message = 'Hook data processed unsuccessfully.';
}
}
else
catch (\Exception $e)
{
$logMessage = 'Uncaught Exception processing issue webhook';

$this->logger->critical($logMessage, ['exception' => $e]);
$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->response->message = 'Hook data processed unsuccessfully.';
}
}
Expand Down Expand Up @@ -127,15 +139,14 @@ protected function insertData()
}
catch (\Exception $e)
{
$this->setStatusCode($e->getCode());
$logMessage = sprintf(
'Error adding GitHub issue %s/%s #%d to the tracker',
$this->project->gh_user,
$this->project->gh_project,
$this->hookData->issue->number
);
$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();

$this->logger->error($logMessage, ['exception' => $e]);

return false;
Expand All @@ -155,7 +166,7 @@ protected function insertData()
'Error processing `onIssueAfterCreate` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -185,7 +196,7 @@ protected function insertData()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -213,7 +224,7 @@ protected function insertData()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -263,7 +274,7 @@ protected function updateData()
$this->project->gh_project,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -330,18 +341,18 @@ protected function updateData()
}
catch (\Exception $e)
{
$this->logger->error(
sprintf(
'Error updating GitHub issue %s/%s #%d (Database ID #%d) in the tracker',
$this->project->gh_user,
$this->project->gh_project,
$this->hookData->issue->number,
$table->id
),
['exception' => $e]
$logMessage = sprintf(
'Error updating GitHub issue %s/%s #%d (Database ID #%d) in the tracker',
$this->project->gh_user,
$this->project->gh_project,
$this->hookData->issue->number,
$table->id
);
$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

$this->getContainer()->get('app')->close();
return false;
}

// Refresh the table object for the listeners
Expand All @@ -357,7 +368,7 @@ protected function updateData()
'Error processing `onIssueAfterUpdate` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -384,7 +395,7 @@ protected function updateData()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -412,7 +423,7 @@ protected function updateData()
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down Expand Up @@ -498,15 +509,14 @@ private function editIssue(IssuesTable $table)
}
catch (\Exception $e)
{
$this->setStatusCode($e->getCode());
$logMessage = sprintf(
'Error editing GitHub issue %s/%s #%d (Database ID #%d) in the tracker',
$this->project->gh_user,
$this->project->gh_project,
$this->hookData->issue->number,
$table->id
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -526,7 +536,7 @@ private function editIssue(IssuesTable $table)
'Error processing `onIssueAfterUpdate` event for issue number %d',
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand All @@ -551,7 +561,7 @@ private function editIssue(IssuesTable $table)
$this->project->project_id,
$this->hookData->issue->number
);

$this->setStatusCode(500);
$this->response->error = $logMessage . ': ' . $e->getMessage();
$this->logger->error($logMessage, ['exception' => $e]);

Expand Down
Loading

0 comments on commit 7708880

Please sign in to comment.