Skip to content

Commit

Permalink
Merge pull request #582 from bugsnag/use-http-client-in-session-tracker
Browse files Browse the repository at this point in the history 
Use HttpClient in SessionTracker
  • Loading branch information
@imjoehaines
imjoehaines authored Jun 5, 2020
2 parents 531fe1c + dd2bd2d commit dfec26c
Show file tree
Hide file tree
Showing 12 changed files with 381 additions and 583 deletions.
35 changes: 35 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
179 changes: 86 additions & 93 deletions src/Client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@

class Client
{
/**
* The default endpoint.
*
* @var string
*/
const ENDPOINT = 'https://notify.bugsnag.com';

/**
* The config instance.
*
Expand Down Expand Up @@ -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();
Expand All @@ -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));
Expand All @@ -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()),
]);
}

/**
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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
*/
Expand All @@ -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
*
Expand All @@ -830,12 +799,36 @@ public function setBuildEndpoint($endpoint)
}

/**
* Returns the build endpoint.
* Get the build endpoint.
*
* @return string
*/
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();
}
}
Loading

0 comments on commit dfec26c

Please sign in to comment.