Add a 15-second timeout to external service calls to fill Doorkeeper link tags
Summary: Depends on D20528. Ref T13291. Ref T13285. Currently, we don't put a timeout on external service calls when enriching URIs for external Asana/JIRA tasks. Add a 15-second timeout so we'll do something reasonable-ish in the face of a downed service provider. Later, I plan to healthcheck Asana/JIRA providers in a generic way (see T13287) so we can stop making calls if they time out / fail too frequently. Test Plan: - Linked to JIRA and Asana tasks in comments. - Set timeout to 0.0001 seconds, saw requests time out. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13291, T13285 Differential Revision: https://secure.phabricator.com/D20530
This commit is contained in:
@@ -5,6 +5,16 @@ abstract class DoorkeeperBridge extends Phobject {
|
|||||||
private $viewer;
|
private $viewer;
|
||||||
private $context = array();
|
private $context = array();
|
||||||
private $throwOnMissingLink;
|
private $throwOnMissingLink;
|
||||||
|
private $timeout;
|
||||||
|
|
||||||
|
public function setTimeout($timeout) {
|
||||||
|
$this->timeout = $timeout;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getTimeout() {
|
||||||
|
return $this->timeout;
|
||||||
|
}
|
||||||
|
|
||||||
public function setThrowOnMissingLink($throw_on_missing_link) {
|
public function setThrowOnMissingLink($throw_on_missing_link) {
|
||||||
$this->throwOnMissingLink = $throw_on_missing_link;
|
$this->throwOnMissingLink = $throw_on_missing_link;
|
||||||
|
|||||||
@@ -62,6 +62,11 @@ final class DoorkeeperBridgeAsana extends DoorkeeperBridge {
|
|||||||
$template = id(new PhutilAsanaFuture())
|
$template = id(new PhutilAsanaFuture())
|
||||||
->setAccessToken($token);
|
->setAccessToken($token);
|
||||||
|
|
||||||
|
$timeout = $this->getTimeout();
|
||||||
|
if ($timeout !== null) {
|
||||||
|
$template->setTimeout($timeout);
|
||||||
|
}
|
||||||
|
|
||||||
$futures = array();
|
$futures = array();
|
||||||
foreach ($id_map as $key => $id) {
|
foreach ($id_map as $key => $id) {
|
||||||
$futures[$key] = id(clone $template)
|
$futures[$key] = id(clone $template)
|
||||||
|
|||||||
@@ -47,12 +47,20 @@ final class DoorkeeperBridgeJIRA extends DoorkeeperBridge {
|
|||||||
// (by querying all instances). For now, just query the one instance.
|
// (by querying all instances). For now, just query the one instance.
|
||||||
$account = head($accounts);
|
$account = head($accounts);
|
||||||
|
|
||||||
|
$timeout = $this->getTimeout();
|
||||||
|
|
||||||
$futures = array();
|
$futures = array();
|
||||||
foreach ($id_map as $key => $id) {
|
foreach ($id_map as $key => $id) {
|
||||||
$futures[$key] = $provider->newJIRAFuture(
|
$future = $provider->newJIRAFuture(
|
||||||
$account,
|
$account,
|
||||||
'rest/api/2/issue/'.phutil_escape_uri($id),
|
'rest/api/2/issue/'.phutil_escape_uri($id),
|
||||||
'GET');
|
'GET');
|
||||||
|
|
||||||
|
if ($timeout !== null) {
|
||||||
|
$future->setTimeout($timeout);
|
||||||
|
}
|
||||||
|
|
||||||
|
$futures[$key] = $future;
|
||||||
}
|
}
|
||||||
|
|
||||||
$results = array();
|
$results = array();
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ final class DoorkeeperTagsController extends PhabricatorController {
|
|||||||
$refs = id(new DoorkeeperImportEngine())
|
$refs = id(new DoorkeeperImportEngine())
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
->setRefs($refs)
|
->setRefs($refs)
|
||||||
|
->setTimeout(15)
|
||||||
->execute();
|
->execute();
|
||||||
|
|
||||||
$results = array();
|
$results = array();
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ final class DoorkeeperImportEngine extends Phobject {
|
|||||||
private $localOnly;
|
private $localOnly;
|
||||||
private $throwOnMissingLink;
|
private $throwOnMissingLink;
|
||||||
private $context = array();
|
private $context = array();
|
||||||
|
private $timeout;
|
||||||
|
|
||||||
public function setViewer(PhabricatorUser $viewer) {
|
public function setViewer(PhabricatorUser $viewer) {
|
||||||
$this->viewer = $viewer;
|
$this->viewer = $viewer;
|
||||||
@@ -43,6 +44,15 @@ final class DoorkeeperImportEngine extends Phobject {
|
|||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setTimeout($timeout) {
|
||||||
|
$this->timeout = $timeout;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getTimeout() {
|
||||||
|
return $this->timeout;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Configure behavior if remote refs can not be retrieved because an
|
* Configure behavior if remote refs can not be retrieved because an
|
||||||
* authentication link is missing.
|
* authentication link is missing.
|
||||||
@@ -98,10 +108,16 @@ final class DoorkeeperImportEngine extends Phobject {
|
|||||||
->setFilterMethod('isEnabled')
|
->setFilterMethod('isEnabled')
|
||||||
->execute();
|
->execute();
|
||||||
|
|
||||||
|
$timeout = $this->getTimeout();
|
||||||
foreach ($bridges as $key => $bridge) {
|
foreach ($bridges as $key => $bridge) {
|
||||||
$bridge->setViewer($viewer);
|
$bridge
|
||||||
$bridge->setThrowOnMissingLink($this->throwOnMissingLink);
|
->setViewer($viewer)
|
||||||
$bridge->setContext($this->context);
|
->setThrowOnMissingLink($this->throwOnMissingLink)
|
||||||
|
->setContext($this->context);
|
||||||
|
|
||||||
|
if ($timeout !== null) {
|
||||||
|
$bridge->setTimeout($timeout);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$working_set = $refs;
|
$working_set = $refs;
|
||||||
|
|||||||
Reference in New Issue
Block a user