diff --git a/CHANGELOG.md b/CHANGELOG.md index 309319b1..5e08e8cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,41 @@ Changelog ========= +## TBD + +### Enhancements + +- The Client and SessionTracker now share a single Guzzle instance (#582) + +### BC breaks + +- Removal of `getSessionClient` method in `Client` and `Configuration` (#582) + This doesn't make sense to keep given the session client is now the same as the notify Guzzle client. Keeping this would either mean changes to the "session" client also propagate to the notify client or changes to this do nothing. Either of which could lead to things being delivered to the wrong endpoint and expected Guzzle changes not being reflected in the actual requests + +- We no longer use the Guzzle base URI for requests (#582) + This means if a Guzzle client is passed in the constructor with a pre-defined base URI, we will still send requests to the notify URI. `Client::setNotifyEndpoint` now needs to be called manually instead. This changed because the Guzzle base URI is ambiguous now given that it is shared between notifications and sessions + +- Removal of `SessionTracker::setConfig` (#582) + This was unused internally and doesn't seem to be needed as the configuration can be changed via the `Client` or `Configuration` instance itself. Having the possibility for there to be two different `Configuration` instances could be dangerous as changes may not propagate as expected + +- `Client::ENDPOINT` (#582) + This is ambiguous as we have three separate endpoints + Use `Configuration::NOTIFY_ENDPOINT` instead + +- `HttpClient::PAYLOAD_VERSION` (#582) + This is ambiguous as there is a session payload version too + Use `HttpClient::NOTIFY_PAYLOAD_VERSION` instead + +- `Report::PAYLOAD_VERSION` (#582) + As above. This was also unused by the notifier + Use `HttpClient::NOTIFY_PAYLOAD_VERSION` instead + +- `SessionTracker::$SESSION_PAYLOAD_VERSION` (#582) + Use `HttpClient::SESSION_PAYLOAD_VERSION` instead + +- `Client::makeGuzzle` has been made private (#582) + Use the `$guzzle` parameter of `Bugsnag\Client::__construct` instead + ## 3.21.0 (2020-04-29) ### Enhancements diff --git a/src/Client.php b/src/Client.php index 72b6509b..1d0857e3 100644 --- a/src/Client.php +++ b/src/Client.php @@ -24,13 +24,6 @@ class Client { - /** - * The default endpoint. - * - * @var string - */ - const ENDPOINT = 'https://notify.bugsnag.com'; - /** * The config instance. * @@ -78,21 +71,34 @@ class Client * * If you don't pass in a key, we'll try to read it from the env variables. * - * @param string|null $apiKey your bugsnag api key - * @param string|null $endpoint your bugsnag endpoint - * @param bool $default if we should register our default callbacks + * @param string|null $apiKey your bugsnag api key + * @param string|null $notifyEndpoint your bugsnag notify endpoint + * @param bool $default if we should register our default callbacks * * @return static */ - public static function make($apiKey = null, $endpoint = null, $defaults = true) - { - // Retrieves environment variables + public static function make( + $apiKey = null, + $notifyEndpoint = null, + $defaults = true + ) { $env = new Env(); - $config = new Configuration($apiKey ?: $env->get('BUGSNAG_API_KEY')); - $guzzle = static::makeGuzzle($endpoint ?: $env->get('BUGSNAG_ENDPOINT')); + if ($apiKey === null) { + $apiKey = $env->get('BUGSNAG_API_KEY'); + } + + $config = new Configuration($apiKey); - $client = new static($config, null, $guzzle); + if ($notifyEndpoint === null) { + $notifyEndpoint = $env->get('BUGSNAG_ENDPOINT'); + } + + if (is_string($notifyEndpoint)) { + $config->setNotifyEndpoint($notifyEndpoint); + } + + $client = new static($config); if ($defaults) { $client->registerDefaultCallbacks(); @@ -102,23 +108,23 @@ public static function make($apiKey = null, $endpoint = null, $defaults = true) } /** - * Create a new client instance. - * - * @param \Bugsnag\Configuration $config - * @param \Bugsnag\Request\ResolverInterface|null $resolver - * @param \GuzzleHttp\ClientInterface|null $guzzle - * @param \Bugsnag\Shutdown\ShutdownStrategyInterface|null $shutdownStrategy - * - * @return void + * @param Configuration $config + * @param ResolverInterface|null $resolver + * @param ClientInterface|null $guzzle + * @param ShutdownStrategyInterface|null $shutdownStrategy */ - public function __construct(Configuration $config, ResolverInterface $resolver = null, ClientInterface $guzzle = null, ShutdownStrategyInterface $shutdownStrategy = null) - { + public function __construct( + Configuration $config, + ResolverInterface $resolver = null, + ClientInterface $guzzle = null, + ShutdownStrategyInterface $shutdownStrategy = null + ) { $this->config = $config; $this->resolver = $resolver ?: new BasicResolver(); $this->recorder = new Recorder(); $this->pipeline = new Pipeline(); - $this->http = new HttpClient($config, $guzzle ?: static::makeGuzzle()); - $this->sessionTracker = new SessionTracker($config); + $this->http = new HttpClient($config, $guzzle ?: self::makeGuzzle()); + $this->sessionTracker = new SessionTracker($config, $this->http); $this->registerMiddleware(new NotificationSkipper($config)); $this->registerMiddleware(new BreadcrumbData($this->recorder)); @@ -130,38 +136,17 @@ public function __construct(Configuration $config, ResolverInterface $resolver = } /** - * Make a new guzzle client instance. - * - * @param string|null $base - * @param array $options - * - * @return \GuzzleHttp\ClientInterface + * @return ClientInterface */ - public static function makeGuzzle($base = null, array $options = []) + private static function makeGuzzle() { - $key = method_exists(ClientInterface::class, 'request') ? 'base_uri' : 'base_url'; - - $options[$key] = $base ?: static::ENDPOINT; - - if ($path = static::getCaBundlePath()) { - $options['verify'] = $path; + if (version_compare(PHP_VERSION, '5.6.0') >= 0) { + return new GuzzleClient(); } - return new GuzzleClient($options); - } - - /** - * Get the ca bundle path if one exists. - * - * @return string|false - */ - protected static function getCaBundlePath() - { - if (version_compare(PHP_VERSION, '5.6.0') >= 0 || !class_exists(CaBundle::class)) { - return false; - } - - return realpath(CaBundle::getSystemCaRootBundlePath()); + return new GuzzleClient([ + 'verify' => realpath(CaBundle::getSystemCaRootBundlePath()), + ]); } /** @@ -323,22 +308,6 @@ public function notify(Report $report, callable $callback = null) } } - /** - * Notify Bugsnag of a deployment. - * - * @deprecated This function is being deprecated in favour of `build`. - * - * @param string|null $repository the repository from which you are deploying the code - * @param string|null $branch the source control branch from which you are deploying - * @param string|null $revision the source control revision you are currently deploying - * - * @return void - */ - public function deploy($repository = null, $branch = null, $revision = null) - { - $this->build($repository, $revision); - } - /** * Notify Bugsnag of a build. * @@ -768,23 +737,33 @@ public function shouldIgnoreErrorCode($code) } /** - * Set session tracking state and pass in optional guzzle. + * Set notification delivery endpoint. * - * @param bool $track whether to track sessions + * @param string $endpoint * * @return $this */ - public function setAutoCaptureSessions($track) + public function setNotifyEndpoint($endpoint) { - $this->config->setAutoCaptureSessions($track); + $this->config->setNotifyEndpoint($endpoint); return $this; } + /** + * Get notification delivery endpoint. + * + * @return string + */ + public function getNotifyEndpoint() + { + return $this->config->getNotifyEndpoint(); + } + /** * Set session delivery endpoint. * - * @param string $endpoint the session endpoint + * @param string $endpoint * * @return $this */ @@ -796,27 +775,17 @@ public function setSessionEndpoint($endpoint) } /** - * Get the session client. - * - * @return \GuzzleHttp\ClientInterface - */ - public function getSessionClient() - { - return $this->config->getSessionClient(); - } - - /** - * Whether should be auto-capturing sessions. + * Get session delivery endpoint. * - * @return bool + * @return string */ - public function shouldCaptureSessions() + public function getSessionEndpoint() { - return $this->config->shouldCaptureSessions(); + return $this->config->getSessionEndpoint(); } /** - * Sets the build endpoint. + * Set the build endpoint. * * @param string $endpoint the build endpoint * @@ -830,7 +799,7 @@ public function setBuildEndpoint($endpoint) } /** - * Returns the build endpoint. + * Get the build endpoint. * * @return string */ @@ -838,4 +807,28 @@ public function getBuildEndpoint() { return $this->config->getBuildEndpoint(); } + + /** + * Set session tracking state. + * + * @param bool $track whether to track sessions + * + * @return $this + */ + public function setAutoCaptureSessions($track) + { + $this->config->setAutoCaptureSessions($track); + + return $this; + } + + /** + * Whether should be auto-capturing sessions. + * + * @return bool + */ + public function shouldCaptureSessions() + { + return $this->config->shouldCaptureSessions(); + } } diff --git a/src/Configuration.php b/src/Configuration.php index 0191b0cf..739bf25b 100644 --- a/src/Configuration.php +++ b/src/Configuration.php @@ -6,23 +6,11 @@ class Configuration { - /** - * The default endpoint. - * - * @var string - */ + const NOTIFY_ENDPOINT = 'https://notify.bugsnag.com'; const SESSION_ENDPOINT = 'https://sessions.bugsnag.com'; - - /** - * The default build endpoint. - * - * @var string - */ const BUILD_ENDPOINT = 'https://build.bugsnag.com'; /** - * The Bugsnag API Key. - * * @var string */ protected $apiKey; @@ -123,25 +111,19 @@ class Configuration protected $autoCaptureSessions = false; /** - * A client to use to send sessions. - * - * @var \GuzzleHttp\ClientInterface + * @var string */ - protected $sessionClient; + protected $notifyEndpoint = self::NOTIFY_ENDPOINT; /** - * The endpoint to deliver sessions to. - * * @var string */ protected $sessionEndpoint = self::SESSION_ENDPOINT; /** - * The endpoint to deliver build notifications to. - * * @var string */ - protected $buildEndpoint; + protected $buildEndpoint = self::BUILD_ENDPOINT; /** * Create a new config instance. @@ -590,23 +572,33 @@ public function shouldIgnoreErrorCode($code) } /** - * Set session tracking state and pass in optional guzzle. + * Set notification delivery endpoint. * - * @param bool $track whether to track sessions + * @param string $endpoint * * @return $this */ - public function setAutoCaptureSessions($track) + public function setNotifyEndpoint($endpoint) { - $this->autoCaptureSessions = $track; + $this->notifyEndpoint = $endpoint; return $this; } + /** + * Get notification delivery endpoint. + * + * @return string + */ + public function getNotifyEndpoint() + { + return $this->notifyEndpoint; + } + /** * Set session delivery endpoint. * - * @param string $endpoint the session endpoint + * @param string $endpoint * * @return $this */ @@ -614,60 +606,64 @@ public function setSessionEndpoint($endpoint) { $this->sessionEndpoint = $endpoint; - $this->sessionClient = Client::makeGuzzle($this->sessionEndpoint); - return $this; } /** - * Get the session client. + * Get session delivery endpoint. * - * @return \GuzzleHttp\ClientInterface + * @return string */ - public function getSessionClient() + public function getSessionEndpoint() { - if (is_null($this->sessionClient)) { - $this->sessionClient = Client::makeGuzzle($this->sessionEndpoint); - } + return $this->sessionEndpoint; + } - return $this->sessionClient; + /** + * Set the build endpoint. + * + * @param string $endpoint the build endpoint + * + * @return $this + */ + public function setBuildEndpoint($endpoint) + { + $this->buildEndpoint = $endpoint; + + return $this; } /** - * Whether should be auto-capturing sessions. + * Get the build endpoint. * - * @return bool + * @return string */ - public function shouldCaptureSessions() + public function getBuildEndpoint() { - return $this->autoCaptureSessions; + return $this->buildEndpoint; } /** - * Sets the build endpoint. + * Set session tracking state. * - * @param string $endpoint the build endpoint + * @param bool $track whether to track sessions * * @return $this */ - public function setBuildEndpoint($endpoint) + public function setAutoCaptureSessions($track) { - $this->buildEndpoint = $endpoint; + $this->autoCaptureSessions = $track; return $this; } /** - * Returns the build endpoint. + * Whether should be auto-capturing sessions. * - * @return string + * @return bool */ - public function getBuildEndpoint() + public function shouldCaptureSessions() { - if (isset($this->buildEndpoint)) { - return $this->buildEndpoint; - } - - return self::BUILD_ENDPOINT; + return $this->autoCaptureSessions; } } diff --git a/src/Env.php b/src/Env.php index 40c03ef7..d1f948c0 100644 --- a/src/Env.php +++ b/src/Env.php @@ -12,9 +12,9 @@ class Env * Copied from phpdotenv: https://github.com/vlucas/phpdotenv/blob/2.6/src/Loader.php#L291. BSD 3-Clause license * provided at this bottom of this file. * - * @param $name + * @param string $name * - * @return array|false|mixed|string|null + * @return mixed */ public function get($name) { diff --git a/src/HttpClient.php b/src/HttpClient.php index c198719b..68b614c2 100644 --- a/src/HttpClient.php +++ b/src/HttpClient.php @@ -9,47 +9,38 @@ class HttpClient { /** - * The config instance. - * - * @var \Bugsnag\Configuration + * The maximum payload size — one megabyte (1024 * 1024). */ - protected $config; + const MAX_SIZE = 1048576; /** - * The guzzle client instance. - * - * @var \GuzzleHttp\ClientInterface + * The payload version for the error notification API. */ - protected $guzzle; + const NOTIFY_PAYLOAD_VERSION = '4.0'; /** - * The queue of reports to send. - * - * @var \Bugsnag\Report[] + * The payload version for the session API. */ - protected $queue = []; + const SESSION_PAYLOAD_VERSION = '1.0'; /** - * The maximum payload size. A whole megabyte (1024 * 1024). - * - * @var int + * @var Configuration */ - const MAX_SIZE = 1048576; + protected $config; /** - * The current payload version. - * - * @var string + * @var ClientInterface */ - const PAYLOAD_VERSION = '4.0'; + protected $guzzle; /** - * Create a new http client instance. - * - * @param \Bugsnag\Configuration $config the configuration instance - * @param \GuzzleHttp\ClientInterface $guzzle the guzzle client instance - * - * @return void + * @var array + */ + protected $queue = []; + + /** + * @param Configuration $config + * @param ClientInterface $guzzle */ public function __construct(Configuration $config, ClientInterface $guzzle) { @@ -60,7 +51,7 @@ public function __construct(Configuration $config, ClientInterface $guzzle) /** * Add a report to the queue. * - * @param \Bugsnag\Report $report the bugsnag report instance + * @param Report $report * * @return void */ @@ -70,27 +61,53 @@ public function queue(Report $report) } /** - * Notify Bugsnag of a deployment. - * - * @deprecated This method should no longer be used in favour of sendBuildReport. - * - * @param array $data the deployment information + * Deliver all errors on the queue to Bugsnag. * * @return void */ - public function deploy(array $data) + public function send() { - $app = $this->config->getAppData(); + if (!$this->queue) { + return; + } - $data['releaseStage'] = $app['releaseStage']; + $events = []; + + foreach ($this->queue as $report) { + $event = $report->toArray(); - if (isset($app['version'])) { - $data['appVersion'] = $app['version']; + if ($event) { + $events[] = $event; + } } - $data['apiKey'] = $this->config->getApiKey(); + $body = [ + 'apiKey' => $this->config->getApiKey(), + 'notifier' => $this->config->getNotifier(), + 'events' => $events, + ]; + + $this->deliverEvents($body); - $this->post('deploy', ['json' => $data]); + $this->queue = []; + } + + /** + * Send a session data payload to Bugsnag. + * + * @param array $payload + * + * @return void + */ + public function sendSessions(array $payload) + { + $this->post( + $this->config->getSessionEndpoint(), + [ + 'json' => $payload, + 'headers' => $this->getHeaders(self::SESSION_PAYLOAD_VERSION), + ] + ); } /** @@ -104,17 +121,16 @@ public function sendBuildReport(array $buildInfo) { $app = $this->config->getAppData(); - $data = []; - $sourceControl = []; - - if (isset($app['version'])) { - $data['appVersion'] = $app['version']; - } else { + if (!isset($app['version'])) { error_log('Bugsnag Warning: App version is not set. Unable to send build report.'); return; } + $data = ['appVersion' => $app['version']]; + + $sourceControl = []; + if (isset($buildInfo['repository'])) { $sourceControl['repository'] = $buildInfo['repository']; } @@ -144,73 +160,72 @@ public function sendBuildReport(array $buildInfo) } $data['releaseStage'] = $app['releaseStage']; - $data['apiKey'] = $this->config->getApiKey(); - $endpoint = $this->config->getBuildEndpoint(); - - $this->post($endpoint, ['json' => $data]); + $this->post($this->config->getBuildEndpoint(), ['json' => $data]); } /** - * Deliver everything on the queue to Bugsnag. + * Deliver the given events to the notification API. * - * @return void - */ - public function send() - { - if (!$this->queue) { - return; - } - - $this->postJson('', $this->build()); - - $this->queue = []; - } - - /** - * Build the request data to send. + * @param array $data * - * @return array + * @return void */ - protected function build() + protected function deliverEvents(array $data) { - $events = []; - - foreach ($this->queue as $report) { - $event = $report->toArray(); + // Try to send the whole lot, or without the meta data for the first + // event. If failed, try to send the first event, and then the rest of + // them, recursively. Decrease by a constant and concquer if you like. + // Note that the base case is satisfied as soon as the payload is small + // enought to send, or when it's simply discarded. + try { + $normalized = $this->normalize($data); + } catch (RuntimeException $e) { + if (count($data['events']) > 1) { + $event = array_shift($data['events']); - if ($event) { - $events[] = $event; + $this->deliverEvents(array_merge($data, ['events' => [$event]])); + $this->deliverEvents($data); + } else { + error_log('Bugsnag Warning: '.$e->getMessage()); } + + return; } - return [ - 'apiKey' => $this->config->getApiKey(), - 'notifier' => $this->config->getNotifier(), - 'events' => $events, - ]; + try { + $this->post( + $this->config->getNotifyEndpoint(), + [ + 'json' => $normalized, + 'headers' => $this->getHeaders(self::NOTIFY_PAYLOAD_VERSION), + ] + ); + } catch (Exception $e) { + error_log('Bugsnag Warning: Couldn\'t notify. '.$e->getMessage()); + } } /** - * Builds the array of headers to send. + * Builds the array of headers to send using the given payload version. + * + * @param string $version The payload version this request is for * * @return array */ - protected function getHeaders() + protected function getHeaders($version) { return [ 'Bugsnag-Api-Key' => $this->config->getApiKey(), 'Bugsnag-Sent-At' => strftime('%Y-%m-%dT%H:%M:%S'), - 'Bugsnag-Payload-Version' => self::PAYLOAD_VERSION, + 'Bugsnag-Payload-Version' => $version, ]; } /** - * Send a POST request to Bugsnag. - * - * @param string $uri the uri to hit - * @param array $data the request options + * @param string $uri + * @param array $options * * @return void */ @@ -223,52 +238,12 @@ protected function post($uri, array $options = []) } } - /** - * Post the given data to Bugsnag in json form. - * - * @param string $uri the uri to hit - * @param array $data the data send - * - * @return void - */ - protected function postJson($uri, array $data) - { - // Try to send the whole lot, or without the meta data for the first - // event. If failed, try to send the first event, and then the rest of - // them, recursively. Decrease by a constant and concquer if you like. - // Note that the base case is satisfied as soon as the payload is small - // enought to send, or when it's simply discarded. - try { - $normalized = $this->normalize($data); - } catch (RuntimeException $e) { - if (count($data['events']) > 1) { - $event = array_shift($data['events']); - $this->postJson($uri, array_merge($data, ['events' => [$event]])); - $this->postJson($uri, $data); - } else { - error_log('Bugsnag Warning: '.$e->getMessage()); - } - - return; - } - - // Send via guzzle and log any failures - try { - $this->post($uri, [ - 'json' => $normalized, - 'headers' => $this->getHeaders(), - ]); - } catch (Exception $e) { - error_log('Bugsnag Warning: Couldn\'t notify. '.$e->getMessage()); - } - } - /** * Normalize the given data to ensure it's the correct size. * - * @param array $data the data to normalize + * @param array $data * - * @throws \RuntimeException + * @throws RuntimeException * * @return array */ @@ -292,7 +267,7 @@ protected function normalize(array $data) /** * Get the length of the given string in bytes. * - * @param string $str the string to get the length of + * @param string $str * * @return int */ diff --git a/src/Report.php b/src/Report.php index bcfaf60d..b231cfaa 100644 --- a/src/Report.php +++ b/src/Report.php @@ -9,17 +9,10 @@ class Report { - /** - * The payload version. - * - * @var string - */ - const PAYLOAD_VERSION = HttpClient::PAYLOAD_VERSION; - /** * The config object. * - * @var \Bugsnag\Config + * @var Configuration */ protected $config; @@ -652,7 +645,7 @@ public function toArray() 'device' => array_merge(['time' => $this->time], $this->config->getDeviceData()), 'user' => $this->getUser(), 'context' => $this->getContext(), - 'payloadVersion' => HttpClient::PAYLOAD_VERSION, + 'payloadVersion' => HttpClient::NOTIFY_PAYLOAD_VERSION, 'severity' => $this->getSeverity(), 'exceptions' => $this->exceptionArray(), 'breadcrumbs' => $this->breadcrumbs, diff --git a/src/SessionTracker.php b/src/SessionTracker.php index 5c42270d..dc2451c0 100644 --- a/src/SessionTracker.php +++ b/src/SessionTracker.php @@ -3,7 +3,6 @@ namespace Bugsnag; use Exception; -use GuzzleHttp\ClientInterface; use InvalidArgumentException; class SessionTracker @@ -18,11 +17,6 @@ class SessionTracker */ protected static $MAX_SESSION_COUNT = 50; - /** - * The current payload version. - */ - protected static $SESSION_PAYLOAD_VERSION = '1.0'; - /** * The key for storing session counts. */ @@ -34,12 +28,15 @@ class SessionTracker protected static $SESSIONS_LAST_SENT_KEY = 'bugsnag-sessions-last-sent'; /** - * The current client configuration. - * * @var Configuration */ protected $config; + /** + * @var HttpClient + */ + protected $http; + /** * An array of session counts. * @@ -87,7 +84,7 @@ class SessionTracker * * @var int */ - protected $lastSent; + protected $lastSent = 0; /** * The current session. @@ -96,27 +93,16 @@ class SessionTracker */ protected $currentSession; - /** - * Create a session tracker instance. - * - * @param Configuration $config the initial client configuration - * - * @return void - */ - public function __construct(Configuration $config) - { - $this->config = $config; - $this->lastSent = 0; - } - /** * @param Configuration $config + * @param HttpClient $http * * @return void */ - public function setConfig(Configuration $config) + public function __construct(Configuration $config, HttpClient $http) { $this->config = $config; + $this->http = $http; } /** @@ -378,30 +364,15 @@ protected function deliverSessions() return; } - $http = $this->config->getSessionClient(); - - $options = [ - 'json' => $this->constructPayload($sessions), - 'headers' => [ - 'Bugsnag-Api-Key' => $this->config->getApiKey(), - 'Bugsnag-Payload-Version' => self::$SESSION_PAYLOAD_VERSION, - 'Bugsnag-Sent-At' => strftime('%Y-%m-%dT%H:%M:%S'), - ], - ]; + $payload = $this->constructPayload($sessions); $this->setLastSent(); try { - // Support later Guzzle versions — note we check the interface to make - // sure "request" is a public method as on PHP 7.4 "method_exists" will - // return true for private methods - if (method_exists(ClientInterface::class, 'request')) { - $http->request('POST', '', $options); - } else { - $http->post('', $options); - } + $this->http->sendSessions($payload); } catch (Exception $e) { error_log('Bugsnag Warning: Couldn\'t notify. '.$e->getMessage()); + if (is_callable($this->retryFunction)) { call_user_func($this->retryFunction, $sessions); } else { diff --git a/tests/ClientTest.php b/tests/ClientTest.php index aef8fa94..5092beac 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -9,7 +9,6 @@ use Bugsnag\Shutdown\PhpShutdownStrategy; use Exception; use GuzzleHttp\Client as Guzzle; -use GuzzleHttp\Psr7\Uri; use Mockery; use ReflectionClass; @@ -31,6 +30,14 @@ protected function setUp() ->getMock(); } + protected function tearDown() + { + putenv('BUGSNAG_API_KEY='); + putenv('BUGSNAG_ENDPOINT='); + unset($_ENV['BUGSNAG_API_KEY']); + unset($_ENV['BUGSNAG_ENDPOINT']); + } + public function testManualErrorNotification() { $this->client->expects($this->once())->method('notify'); @@ -77,77 +84,41 @@ public function testManualExceptionNotificationWithSeverity() }); } - protected function getGuzzle(Client $client) + public function testTheNotifyEndpointHasASensibleDefault() { - $prop = (new ReflectionClass($client))->getProperty('http'); - $prop->setAccessible(true); - - $http = $prop->getValue($client); + $client = Client::make('123'); + $expected = 'https://notify.bugsnag.com'; - $prop = (new ReflectionClass($http))->getProperty('guzzle'); - $prop->setAccessible(true); - - return $prop->getValue($http); - } - - public function testDefaultSetup() - { - $this->assertEquals( - new Uri('https://notify.bugsnag.com'), - self::getGuzzleBaseUri($this->getGuzzle(Client::make('123'))) - ); + $this->assertEquals($expected, $client->getNotifyEndpoint()); } - public function testCanMake() + public function testTheNotifyEndpointCanBeSetByPassingItToMake() { $client = Client::make('123', 'https://example.com'); - $this->assertInstanceOf(Client::class, $client); - - $this->assertEquals( - new Uri('https://example.com'), - self::getGuzzleBaseUri($this->getGuzzle($client)) - ); + $this->assertEquals('https://example.com', $client->getNotifyEndpoint()); } - public function testCanMakeFromEnv() + public function testTheApiKeyAndNotifyEndpointCanBeSetViaEnvironmentVariables() { - try { - putenv('BUGSNAG_API_KEY=foo-baz'); - putenv('BUGSNAG_ENDPOINT=http://foo.com'); - - $client = Client::make(); + putenv('BUGSNAG_API_KEY=foobar'); + putenv('BUGSNAG_ENDPOINT=http://foo.com'); - $this->assertInstanceOf(Client::class, $client); + $client = Client::make(); - $this->assertEquals( - new Uri('http://foo.com'), - self::getGuzzleBaseUri($this->getGuzzle($client)) - ); - } finally { - putenv('BUGSNAG_API_KEY='); - putenv('BUGSNAG_ENDPOINT='); - } + $this->assertEquals('foobar', $client->getApiKey()); + $this->assertEquals('http://foo.com', $client->getNotifyEndpoint()); } - public function testCanMakeFromEnvSuperglobal() + public function testTheApiKeyAndNotifyEndpointCanBeSetViaEnvSuperglobal() { - try { - $_ENV['BUGSNAG_API_KEY'] = 'foo-bar'; - $_ENV['BUGSNAG_ENDPOINT'] = 'http://bar.com'; - - $client = Client::make(); + $_ENV['BUGSNAG_API_KEY'] = 'baz'; + $_ENV['BUGSNAG_ENDPOINT'] = 'http://bar.com'; - $this->assertInstanceOf(Client::class, $client); + $client = Client::make(); - $this->assertEquals( - new Uri('http://bar.com'), - self::getGuzzleBaseUri($this->getGuzzle($client)) - ); - } finally { - unset($_ENV['BUGSNAG_API_KEY']); - unset($_ENV['BUGSNAG_ENDPOINT']); - } + $this->assertEquals('baz', $client->getApiKey()); + $this->assertEquals('http://bar.com', $client->getNotifyEndpoint()); } public function testBeforeNotifySkipsError() @@ -509,99 +480,6 @@ public function testCanManuallyFlush() $this->client->flush(); } - public function testDeployWorksOutOfTheBox() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['releaseStage' => 'production', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy(); - } - - public function testDeployWorksWithReleaseStage() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['releaseStage' => 'staging', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - $this->config->setReleaseStage('staging'); - - $this->client->deploy(); - } - - public function testDeployWorksWithAppVersion() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['releaseStage' => 'production', 'appVersion' => '1.1.0', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy(); - } - - public function testDeployWorksWithRepository() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['sourceControl' => ['repository' => 'foo'], 'releaseStage' => 'production', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy('foo'); - } - - public function testDeployWorksWithBranch() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['releaseStage' => 'production', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy(null, 'master'); - } - - public function testDeployWorksWithRevision() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['sourceControl' => ['revision' => 'bar'], 'releaseStage' => 'production', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy(null, null, 'bar'); - } - - public function testDeployWorksWithEverything() - { - $this->guzzlePostWith( - 'https://build.bugsnag.com', - ['json' => ['sourceControl' => ['repository' => 'baz', 'revision' => 'foo'], 'releaseStage' => 'development', 'appVersion' => '1.3.1', 'apiKey' => 'example-api-key', 'buildTool' => 'bugsnag-php', 'builderName' => exec('whoami'), 'appVersion' => '1.3.1']] - ); - - $this->client = new Client($this->config = new Configuration('example-api-key'), null, $this->guzzle); - $this->config->setReleaseStage('development'); - $this->config->setAppVersion('1.3.1'); - - $this->client->deploy('baz', 'develop', 'foo'); - } - public function testBuildWorksOutOfTheBox() { $this->guzzlePostWith( @@ -871,12 +749,22 @@ public function testBuildEndpoint() $this->assertSame('https://example', $client->getBuildEndpoint()); } - public function testSessionClient() + public function testTheSessionEndpointHasASensibleDefault() + { + $client = Client::make('foo'); + $expected = 'https://sessions.bugsnag.com'; + + $this->assertSame($expected, $client->getSessionEndpoint()); + } + + public function testTheSessionEndpointCanBeSetIfNecessary() { $client = Client::make('foo'); - $this->assertSame($client, $client->setSessionEndpoint('https://example')); - $sessionClient = $client->getSessionClient(); - $this->assertEquals(new Uri('https://example'), self::getGuzzleBaseUri($sessionClient)); + $expected = 'https://example.com'; + + $client->setSessionEndpoint($expected); + + $this->assertSame($expected, $client->getSessionEndpoint()); } public function testSetAutoCaptureSessions() diff --git a/tests/ConfigurationTest.php b/tests/ConfigurationTest.php index 0ec39b81..6da2a35a 100644 --- a/tests/ConfigurationTest.php +++ b/tests/ConfigurationTest.php @@ -3,8 +3,6 @@ namespace Bugsnag\Tests; use Bugsnag\Configuration; -use GuzzleHttp\Client as GuzzleClient; -use GuzzleHttp\Psr7\Uri; class ConfigurationTest extends TestCase { @@ -278,42 +276,31 @@ public function testMergeDeviceDataComplexValues() $this->assertSame(['f1' => 1], $this->config->getDeviceData()['assoc_array_field']); } - public function testSessionTrackingDefaults() + public function testSessionTrackingIsDisabledByDefault() { $this->assertFalse($this->config->shouldCaptureSessions()); } - public function testSessionTrackingSetTrue() + public function testSessionTrackingCanBeEnabled() { - $this->assertFalse($this->config->shouldCaptureSessions()); - $this->config->setAutoCaptureSessions(true); $this->assertTrue($this->config->shouldCaptureSessions()); - - $client = $this->config->getSessionClient(); - - $this->assertSame(GuzzleClient::class, get_class($client)); - - $this->assertEquals(new Uri(Configuration::SESSION_ENDPOINT), self::getGuzzleBaseUri($client)); } - public function testSessionTrackingSetEndpoint() + public function testTheSessionEndpointHasASensibleDefault() { - $testUrl = 'https://testurl.com'; - - $this->assertFalse($this->config->shouldCaptureSessions()); + $expected = 'https://sessions.bugsnag.com'; - $this->config->setAutoCaptureSessions(true); - - $this->assertTrue($this->config->shouldCaptureSessions()); - - $this->config->setSessionEndpoint($testUrl); + $this->assertSame($expected, $this->config->getSessionEndpoint()); + } - $client = $this->config->getSessionClient(); + public function testTheSessionEndpointCanBeSetIfNecessary() + { + $expected = 'https://example.com'; - $this->assertSame(GuzzleClient::class, get_class($client)); + $this->config->setSessionEndpoint($expected); - $this->assertEquals(new Uri($testUrl), self::getGuzzleBaseUri($client)); + $this->assertSame($expected, $this->config->getSessionEndpoint()); } } diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 7bfae627..2941ed63 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -46,7 +46,7 @@ public function testHttpClient() $this->assertCount(1, $invocations = $spy->getInvocations()); $params = self::getInvocationParameters($invocations[0]); $this->assertCount(self::getGuzzleExpectedParamCount(), $params); - $this->assertSame('', self::getGuzzlePostUriParam($params)); + $this->assertSame($this->config->getNotifyEndpoint(), self::getGuzzlePostUriParam($params)); $options = self::getGuzzlePostOptionsParam($params); $this->assertInternalType('array', $options); $this->assertInternalType('array', $options['json']['notifier']); @@ -91,7 +91,7 @@ public function testMassiveMetaDataHttpClient() $this->assertCount(1, $invocations = $spy->getInvocations()); $params = self::getInvocationParameters($invocations[0]); $this->assertCount(self::getGuzzleExpectedParamCount(), $params); - $this->assertSame('', self::getGuzzlePostUriParam($params)); + $this->assertSame($this->config->getNotifyEndpoint(), self::getGuzzlePostUriParam($params)); $options = self::getGuzzlePostOptionsParam($params); $this->assertInternalType('array', $options); $this->assertInternalType('array', $options['json']['notifier']); @@ -140,7 +140,7 @@ public function testPartialHttpClient() $this->assertCount(1, $invocations = $spy->getInvocations()); $params = self::getInvocationParameters($invocations[0]); $this->assertCount(self::getGuzzleExpectedParamCount(), $params); - $this->assertSame('', self::getGuzzlePostUriParam($params)); + $this->assertSame($this->config->getNotifyEndpoint(), self::getGuzzlePostUriParam($params)); $options = self::getGuzzlePostOptionsParam($params); $this->assertInternalType('array', $options); $this->assertInternalType('array', $options['json']['notifier']); diff --git a/tests/SessionTrackerTest.php b/tests/SessionTrackerTest.php index daa67ef1..2afad633 100644 --- a/tests/SessionTrackerTest.php +++ b/tests/SessionTrackerTest.php @@ -3,8 +3,8 @@ namespace Bugsnag\Tests; use Bugsnag\Configuration; +use Bugsnag\HttpClient; use Bugsnag\SessionTracker; -use GuzzleHttp\ClientInterface; use InvalidArgumentException; use PHPUnit\Framework\MockObject\MockObject; use RuntimeException; @@ -16,8 +16,8 @@ class SessionTrackerTest extends TestCase private $sessionTracker; /** @var Configuration&MockObject */ private $config; - /** @var ClientInterface&MockObject */ - private $guzzleClient; + /** @var HttpClient&MockObject */ + private $client; public function setUp() { @@ -26,15 +26,16 @@ public function setUp() ->setConstructorArgs(['example-api-key']) ->getMock(); - $this->sessionTracker = new SessionTracker($this->config); - - $this->guzzleClient = $this->getMockBuilder(ClientInterface::class) + $this->client = $this->getMockBuilder(HttpClient::class) + ->disableOriginalConstructor() ->getMock(); + + $this->sessionTracker = new SessionTracker($this->config, $this->client); } public function testSendSessionsEmpty() { - $this->config->expects($this->never())->method('getSessionClient'); + $this->client->expects($this->never())->method('sendSessions'); $this->sessionTracker->sendSessions(); } @@ -57,7 +58,7 @@ public function testSendSessionsShouldNotNotify() }); $this->config->expects($this->once())->method('shouldNotify')->willReturn(false); - $this->config->expects($this->never())->method('getSessionClient'); + $this->client->expects($this->never())->method('sendSessions'); $this->sessionTracker->sendSessions(); @@ -78,12 +79,11 @@ public function testSendSessionsReturnsEarlyWhenGetSessionCountsReturnsAValueTha }); $this->config->expects($this->never())->method('shouldNotify'); - $this->config->expects($this->never())->method('getSessionClient'); $this->config->expects($this->never())->method('getNotifier'); $this->config->expects($this->never())->method('getDeviceData'); $this->config->expects($this->never())->method('getAppData'); $this->config->expects($this->never())->method('getApiKey'); - $this->guzzleClient->expects($this->never())->method($this->getGuzzleMethod()); + $this->client->expects($this->never())->method('sendSessions'); $this->sessionTracker->sendSessions(); } @@ -107,12 +107,11 @@ public function testStartSessionDoesNotDeliverSessionsWhenLastSentIsNotAnInteger }); $this->config->expects($this->never())->method('shouldNotify'); - $this->config->expects($this->never())->method('getSessionClient'); $this->config->expects($this->never())->method('getNotifier'); $this->config->expects($this->never())->method('getDeviceData'); $this->config->expects($this->never())->method('getAppData'); $this->config->expects($this->never())->method('getApiKey'); - $this->guzzleClient->expects($this->never())->method($this->getGuzzleMethod()); + $this->client->expects($this->never())->method('sendSessions'); $this->sessionTracker->startSession(); } @@ -131,12 +130,11 @@ public function testStartSessionDoesNotDeliverSessionsWhenGetSessionCountsReturn }); $this->config->expects($this->never())->method('shouldNotify'); - $this->config->expects($this->never())->method('getSessionClient'); $this->config->expects($this->never())->method('getNotifier'); $this->config->expects($this->never())->method('getDeviceData'); $this->config->expects($this->never())->method('getAppData'); $this->config->expects($this->never())->method('getApiKey'); - $this->guzzleClient->expects($this->never())->method($this->getGuzzleMethod()); + $this->client->expects($this->never())->method('sendSessions'); $this->sessionTracker->startSession(); } @@ -161,34 +159,23 @@ public function testSendSessionsSuccess() }); $this->config->expects($this->once())->method('shouldNotify')->willReturn(true); - $this->config->expects($this->once())->method('getSessionClient')->willReturn($this->guzzleClient); $this->config->expects($this->once())->method('getNotifier')->willReturn('test_notifier'); $this->config->expects($this->once())->method('getDeviceData')->willReturn('device_data'); $this->config->expects($this->once())->method('getAppData')->willReturn('app_data'); - $this->config->expects($this->once())->method('getApiKey')->willReturn('example-api-key'); - - $expectCallback = function ($sessionPayload) { - return count($sessionPayload) == 2 - && $sessionPayload['json']['notifier'] === 'test_notifier' - && $sessionPayload['json']['device'] === 'device_data' - && $sessionPayload['json']['app'] === 'app_data' - && count($sessionPayload['json']['sessionCounts']) === 1 - && $sessionPayload['json']['sessionCounts'][0]['startedAt'] === '2000-01-01T00:00:00' - && $sessionPayload['json']['sessionCounts'][0]['sessionsStarted'] === 1 - && $sessionPayload['headers']['Bugsnag-Api-Key'] == 'example-api-key' - && preg_match('/(\d+\.)+/', $sessionPayload['headers']['Bugsnag-Payload-Version']) - && preg_match('/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/', $sessionPayload['headers']['Bugsnag-Sent-At']); + + $expectCallback = function ($payload) { + return count($payload) == 4 + && $payload['notifier'] === 'test_notifier' + && $payload['device'] === 'device_data' + && $payload['app'] === 'app_data' + && count($payload['sessionCounts']) === 1 + && $payload['sessionCounts'][0]['startedAt'] === '2000-01-01T00:00:00' + && $payload['sessionCounts'][0]['sessionsStarted'] === 1; }; - if ($this->getGuzzleMethod() === 'post') { - $this->guzzleClient->expects($this->once()) - ->method($this->getGuzzleMethod()) - ->with('', $this->callback($expectCallback)); - } else { - $this->guzzleClient->expects($this->once()) - ->method($this->getGuzzleMethod()) - ->with('POST', '', $this->callback($expectCallback)); - } + $this->client->expects($this->once()) + ->method('sendSessions') + ->with($this->callback($expectCallback)); $this->sessionTracker->sendSessions(); } @@ -210,34 +197,26 @@ public function testStartSessionsSuccess() }); $this->config->expects($this->once())->method('shouldNotify')->willReturn(true); - $this->config->expects($this->once())->method('getSessionClient')->willReturn($this->guzzleClient); $this->config->expects($this->once())->method('getNotifier')->willReturn('test_notifier'); $this->config->expects($this->once())->method('getDeviceData')->willReturn('device_data'); $this->config->expects($this->once())->method('getAppData')->willReturn('app_data'); - $this->config->expects($this->once())->method('getApiKey')->willReturn('example-api-key'); - - $expectCallback = function ($sessionPayload) { - return count($sessionPayload) == 2 - && $sessionPayload['json']['notifier'] === 'test_notifier' - && $sessionPayload['json']['device'] === 'device_data' - && $sessionPayload['json']['app'] === 'app_data' - && count($sessionPayload['json']['sessionCounts']) === 1 - && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/', $sessionPayload['json']['sessionCounts'][0]['startedAt']) - && $sessionPayload['json']['sessionCounts'][0]['sessionsStarted'] === 1 - && $sessionPayload['headers']['Bugsnag-Api-Key'] == 'example-api-key' - && preg_match('/(\d+\.)+/', $sessionPayload['headers']['Bugsnag-Payload-Version']) - && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/', $sessionPayload['headers']['Bugsnag-Sent-At']); + + $expectCallback = function ($payload) { + return count($payload) == 4 + && $payload['notifier'] === 'test_notifier' + && $payload['device'] === 'device_data' + && $payload['app'] === 'app_data' + && count($payload['sessionCounts']) === 1 + && preg_match( + '/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/', + $payload['sessionCounts'][0]['startedAt'] + ) + && $payload['sessionCounts'][0]['sessionsStarted'] === 1; }; - if ($this->getGuzzleMethod() === 'post') { - $this->guzzleClient->expects($this->once()) - ->method($this->getGuzzleMethod()) - ->with('', $this->callback($expectCallback)); - } else { - $this->guzzleClient->expects($this->once()) - ->method($this->getGuzzleMethod()) - ->with('POST', '', $this->callback($expectCallback)); - } + $this->client->expects($this->once()) + ->method('sendSessions') + ->with($this->callback($expectCallback)); $this->sessionTracker->startSession(); } @@ -295,7 +274,6 @@ function () use (&$locked, &$unlockWasCalled) { }); $this->config->expects($this->once())->method('shouldNotify')->willReturn(true); - $this->config->expects($this->once())->method('getSessionClient')->willReturn($this->guzzleClient); $this->sessionTracker->startSession(); diff --git a/tests/TestCase.php b/tests/TestCase.php index 78724acc..d2ef2db7 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -3,9 +3,7 @@ namespace Bugsnag\Tests; use GrahamCampbell\TestBenchCore\MockeryTrait; -use GuzzleHttp\Client as Guzzle; use GuzzleHttp\ClientInterface; -use GuzzleHttp\Psr7\Uri; use phpmock\phpunit\PHPMock; use PHPUnit\Framework\TestCase as BaseTestCase; use ReflectionObject; @@ -38,22 +36,6 @@ protected static function getGuzzleMethod() return method_exists(ClientInterface::class, 'request') ? 'request' : 'post'; } - /** - * @param \GuzzleHttp\Client $guzzle - * - * @return GuzzleHttp\Psr7\Uri|null - */ - protected static function getGuzzleBaseUri(Guzzle $guzzle) - { - if (method_exists($guzzle, 'getBaseUrl')) { - return new Uri($guzzle->getBaseUrl()); - } - - $config = self::readObjectAttribute($guzzle, 'config'); - - return isset($config['base_uri']) ? $config['base_uri'] : null; - } - private static function readObjectAttribute($object, $attributeName) { $reflector = new ReflectionObject($object);