Skip to content

Commit 48f173c

Browse files
odinuvodinuv
authored
Merge pull request #16 from keboola/odin-PS-3840
log retries
2 parents 4923002 + e1f3afa commit 48f173c

28 files changed

Lines changed: 510 additions & 171 deletions

‎.env‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
STORAGE_API_URL=
2+
TEST_MANAGE_API_APPLICATION_TOKEN=
3+
TEST_NOTIFICATION_API_URL=
4+
TEST_STORAGE_API_PROJECT_ID=
5+
TEST_STORAGE_API_TOKEN=

‎.gitignore‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
vendor/
22
.idea/
3-
composer.lock
43
/.php_cs.cache
54
/build/logs
65
/infection.log
76
/.phpunit.result.cache
87
/set-env.sh
98
/provisioning/environments.yaml
9+
/.env.local
10+
/.env.local.php
11+
/.env.*.local

‎composer.json‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,28 @@
2525
}
2626
},
2727
"require": {
28-
"php": "^7.4",
28+
"php": "^7.4|^8.0",
2929
"ext-json": "*",
3030
"guzzlehttp/guzzle": "^6.3|^7.2",
3131
"myclabs/php-enum": "^1.8",
3232
"psr/log": "^1.1",
33-
"symfony/validator": "^5.2"
33+
"symfony/validator": "^5.2|^6.0"
3434
},
3535
"require-dev": {
36-
"infection/infection": "^0.18.2|^0.21.2",
37-
"keboola/coding-standard": "^12.0",
38-
"php-parallel-lint/php-parallel-lint": "^1.2",
39-
"phpstan/phpstan": "^0.12.80",
40-
"phpunit/phpunit": "^9.5"
36+
"infection/infection": "^0.26",
37+
"keboola/coding-standard": ">=14.0",
38+
"phpstan/phpstan": "^1.8",
39+
"phpstan/phpstan-phpunit": "^1.1",
40+
"phpunit/phpunit": "^9.5",
41+
"symfony/dotenv": "^5.2|^6.0"
4142
},
4243
"scripts": {
4344
"tests": "phpunit --coverage-clover build/logs/clover.xml --coverage-xml=build/logs/coverage-xml --log-junit=build/logs/phpunit.junit.xml",
4445
"phpstan": "phpstan analyse --no-progress -c phpstan.neon",
4546
"phpcs": "phpcs --extensions=php src tests",
4647
"phpcbf": "phpcbf --extensions=php src tests",
47-
"phplint": "parallel-lint -j 10 --exclude vendor .",
4848
"infection": "infection --threads=4 --min-covered-msi=90 --coverage=build/logs",
4949
"build": [
50-
"@phplint",
5150
"@phpcs",
5251
"@phpstan",
5352
"@tests",
@@ -61,8 +60,10 @@
6160
"config": {
6261
"sort-packages": true,
6362
"process-timeout": 3600,
63+
"lock": false,
6464
"allow-plugins": {
65-
"infection/extension-installer": true
65+
"infection/extension-installer": true,
66+
"dealerdirect/phpcodesniffer-composer-installer": true
6667
}
6768
}
6869
}

‎phpcs.xml‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
<?xml version="1.0"?>
22
<ruleset name="Project">
3-
<rule ref="vendor/keboola/coding-standard/src/ruleset.xml"/>
4-
</ruleset>
3+
<rule ref="vendor/keboola/coding-standard/src/ruleset.xml">
4+
<exclude name="SlevomatCodingStandard.TypeHints.ParameterTypeHint"/>
5+
<exclude name="SlevomatCodingStandard.TypeHints.PropertyTypeHint"/>
6+
<exclude name="SlevomatCodingStandard.TypeHints.ReturnTypeHint"/>
7+
</rule>
8+
<exclude-pattern>*/src/Kernel.php</exclude-pattern>
9+
</ruleset>

‎phpstan.neon‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,9 @@ parameters:
44
paths:
55
- src
66
- tests
7+
stubFiles:
8+
- tests/stubs/Guzzle.stub
79
ignoreErrors:
10+
11+
includes:
12+
- vendor/phpstan/phpstan-phpunit/extension.neon

‎src/Client.php‎

Lines changed: 74 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use Closure;
88
use GuzzleHttp\Client as GuzzleClient;
9-
use GuzzleHttp\Exception\ClientException;
109
use GuzzleHttp\Exception\GuzzleException;
1110
use GuzzleHttp\HandlerStack;
1211
use GuzzleHttp\MessageFormatter;
@@ -18,35 +17,50 @@
1817
use Psr\Http\Message\RequestInterface;
1918
use Psr\Http\Message\ResponseInterface;
2019
use Psr\Log\LoggerInterface;
20+
use Psr\Log\NullLogger;
21+
use Symfony\Component\Validator\Constraints\NotBlank;
2122
use Symfony\Component\Validator\Constraints\Range;
2223
use Symfony\Component\Validator\Constraints\Url;
2324
use Symfony\Component\Validator\ConstraintViolationInterface;
2425
use Symfony\Component\Validator\Validation;
26+
use Throwable;
2527

2628
abstract class Client
2729
{
28-
private const DEFAULT_USER_AGENT = 'Notification PHP Client';
29-
private const DEFAULT_BACKOFF_RETRIES = 3;
3030
private const JSON_DEPTH = 512;
31-
31+
protected string $tokenHeaderName = '';
3232
protected GuzzleClient $guzzle;
3333

34+
/**
35+
* @param array{
36+
* handler?: HandlerStack,
37+
* backoffMaxTries: int<0, 100>,
38+
* userAgent: string,
39+
* logger?: LoggerInterface
40+
* } $options
41+
*/
3442
public function __construct(
3543
string $baseUrl,
3644
?string $token,
37-
array $options = []
45+
array $options
3846
) {
3947
$validator = Validation::createValidator();
4048
$errors = $validator->validate($baseUrl, [new Url()]);
41-
if (!empty($options['backoffMaxTries'])) {
42-
$errors->addAll($validator->validate($options['backoffMaxTries'], [new Range(['min' => 0, 'max' => 100])]));
43-
$options['backoffMaxTries'] = intval($options['backoffMaxTries']);
44-
} else {
45-
$options['backoffMaxTries'] = self::DEFAULT_BACKOFF_RETRIES;
46-
}
47-
if (empty($options['userAgent'])) {
48-
$options['userAgent'] = self::DEFAULT_USER_AGENT;
49-
}
49+
$errors->addAll($validator->validate(
50+
// @phpstan-ignore-next-line
51+
$options['backoffMaxTries'] ?? null,
52+
[new NotBlank(null, '"backoffMaxTries" option must be provided')]
53+
));
54+
$errors->addAll($validator->validate(
55+
// @phpstan-ignore-next-line
56+
$options['backoffMaxTries'] ?? null,
57+
[new Range(['min' => 0, 'max' => 100])]
58+
));
59+
$errors->addAll($validator->validate(
60+
// @phpstan-ignore-next-line
61+
$options['userAgent'] ?? null,
62+
[new NotBlank(null, '"userAgent" option must be provided')]
63+
));
5064
if ($errors->count() !== 0) {
5165
$messages = '';
5266
/** @var ConstraintViolationInterface $error */
@@ -55,63 +69,86 @@ public function __construct(
5569
}
5670
throw new NotificationClientException('Invalid parameters when creating client: ' . $messages);
5771
}
72+
5873
$this->guzzle = $this->initClient($baseUrl, $token, $options);
5974
}
6075

61-
private function createDefaultDecider(int $maxRetries): Closure
76+
private function createDefaultDecider(int $maxRetries, LoggerInterface $logger): Closure
6277
{
6378
return function (
6479
$retries,
6580
RequestInterface $request,
6681
?ResponseInterface $response = null,
67-
$error = null
68-
) use ($maxRetries) {
82+
?Throwable $error = null
83+
) use (
84+
$maxRetries,
85+
$logger
86+
) {
6987
if ($retries >= $maxRetries) {
88+
$logger->notice(sprintf('We have tried this %d times. Giving up.', $maxRetries));
7089
return false;
90+
} elseif ($response && $response->getStatusCode() >= 500) {
91+
$logger->notice(sprintf(
92+
'Got a %s response for this reason: %s, retrying.',
93+
$response->getStatusCode(),
94+
$response->getReasonPhrase()
95+
));
96+
return true;
7197
} elseif ($error && $error->getCode() >= 500) {
98+
$logger->notice(sprintf(
99+
'Got a %s error with this message: %s, retrying.',
100+
$error->getCode(),
101+
$error->getMessage()
102+
));
72103
return true;
73104
} else {
74105
return false;
75106
}
76107
};
77108
}
78109

79-
abstract protected function getTokenHeaderName(): ?string;
80-
81-
private function initClient(string $url, ?string $token, array $options = []): GuzzleClient
110+
/**
111+
* @param array{
112+
* handler?: HandlerStack,
113+
* backoffMaxTries: int<0, 100>,
114+
* userAgent: string,
115+
* logger?: LoggerInterface
116+
* } $options
117+
*/
118+
private function initClient(string $url, ?string $token, array $options): GuzzleClient
82119
{
83120
// Initialize handlers (start with those supplied in constructor)
84121
$handlerStack = HandlerStack::create($options['handler'] ?? null);
85-
// Set exponential backoff
86-
$handlerStack->push(Middleware::retry($this->createDefaultDecider($options['backoffMaxTries'])));
87-
// Set handler to set default headers
88-
$handlerStack->push(Middleware::mapRequest(
89-
function (RequestInterface $request) use ($token, $options) {
90-
$request = $request
91-
->withHeader('User-Agent', $options['userAgent'])
92-
->withHeader('Content-type', 'application/json');
93-
if ($this->getTokenHeaderName()) {
94-
$request = $request->withHeader((string) $this->getTokenHeaderName(), (string) $token);
95-
}
96-
return $request;
97-
}
98-
));
122+
99123
// Set client logger
100-
if (isset($options['logger']) && $options['logger'] instanceof LoggerInterface) {
124+
if (isset($options['logger'])) {
101125
$handlerStack->push(Middleware::log(
102126
$options['logger'],
103127
new MessageFormatter(
104128
'{hostname} {req_header_User-Agent} - [{ts}] "{method} {resource} {protocol}/{version}"' .
105129
' {code} {res_header_Content-Length}'
106-
)
130+
),
131+
'debug'
107132
));
133+
$logger = $options['logger'];
134+
} else {
135+
$logger = new NullLogger();
108136
}
137+
138+
// Set exponential backoff
139+
$handlerStack->push(Middleware::retry($this->createDefaultDecider($options['backoffMaxTries'], $logger)));
140+
$headers['User-Agent'] = $options['userAgent'];
141+
if ($this->tokenHeaderName) {
142+
$headers[$this->tokenHeaderName] = (string) $token;
143+
}
144+
109145
// finally create the instance
110146
return new GuzzleClient([
111147
'base_uri' => $url,
112148
'handler' => $handlerStack,
113149
RequestOptions::CONNECT_TIMEOUT => 10,
114150
RequestOptions::TIMEOUT => 120,
151+
'headers' => $headers,
115152
]);
116153
}
117154

@@ -123,9 +160,7 @@ protected function sendRequest(Request $request): array
123160
if ($body === '') {
124161
return [];
125162
}
126-
return json_decode($body, true, self::JSON_DEPTH, JSON_THROW_ON_ERROR);
127-
} catch (ClientException $e) {
128-
throw new NotificationClientException($e->getMessage(), $e->getCode(), $e);
163+
return (array) json_decode($body, true, self::JSON_DEPTH, JSON_THROW_ON_ERROR);
129164
} catch (GuzzleException $e) {
130165
throw new NotificationClientException($e->getMessage(), $e->getCode(), $e);
131166
} catch (JsonException $e) {

‎src/ClientFactory.php‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,33 @@
44

55
namespace Keboola\NotificationClient;
66

7-
use Keboola\NotificationClient\Requests\PostEvent\JobFailedEventData;
7+
use GuzzleHttp\HandlerStack;
8+
use Psr\Log\LoggerInterface;
89

910
class ClientFactory
1011
{
1112
private const NOTIFICATION_SERVICE_NAME = 'notification';
1213
private ?string $notificationUrl;
1314
private string $connectionUrl;
15+
/**
16+
* @var array{
17+
* handler?: HandlerStack,
18+
* backoffMaxTries: int<0, 100>,
19+
* userAgent: string,
20+
* logger?: LoggerInterface
21+
* }
22+
*/
1423
private array $connectionClientOptions;
1524

16-
public function __construct(string $connectionUrl, array $connectionClientOptions = [])
25+
/**
26+
* @param array{
27+
* handler?: HandlerStack,
28+
* backoffMaxTries: int<0, 100>,
29+
* userAgent: string,
30+
* logger?: LoggerInterface
31+
* } $connectionClientOptions
32+
*/
33+
public function __construct(string $connectionUrl, array $connectionClientOptions)
1734
{
1835
$this->connectionUrl = $connectionUrl;
1936
$this->connectionClientOptions = $connectionClientOptions;
@@ -29,12 +46,28 @@ private function getNotificationUrl(): string
2946
return (string) $this->notificationUrl;
3047
}
3148

32-
public function getEventsClient(string $applicationToken, array $options = []): EventsClient
49+
/**
50+
* @param array{
51+
* handler?: HandlerStack,
52+
* backoffMaxTries: int<0, 100>,
53+
* userAgent: string,
54+
* logger?: LoggerInterface
55+
* } $options
56+
*/
57+
public function getEventsClient(string $applicationToken, array $options): EventsClient
3358
{
3459
return new EventsClient($this->getNotificationUrl(), $applicationToken, $options);
3560
}
3661

37-
public function getSubscriptionClient(string $storageApiToken, array $options = []): SubscriptionClient
62+
/**
63+
* @param array{
64+
* handler?: HandlerStack,
65+
* backoffMaxTries: int<0, 100>,
66+
* userAgent: string,
67+
* logger?: LoggerInterface
68+
* } $options
69+
*/
70+
public function getSubscriptionClient(string $storageApiToken, array $options): SubscriptionClient
3871
{
3972
return new SubscriptionClient($this->getNotificationUrl(), $storageApiToken, $options);
4073
}

‎src/EventsClient.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,11 @@
1111

1212
class EventsClient extends Client
1313
{
14-
protected function getTokenHeaderName(): string
15-
{
16-
return 'X-Kbc-ManageApiToken';
17-
}
14+
protected string $tokenHeaderName = 'X-Kbc-ManageApiToken';
1815

19-
public function __construct(
20-
string $notificationApiUrl,
21-
string $applicationToken,
22-
array $options = []
23-
) {
16+
/** @inheritDoc */
17+
public function __construct(string $notificationApiUrl, string $applicationToken, array $options)
18+
{
2419
if (empty($applicationToken)) {
2520
throw new ClientException(sprintf(
2621
'Application token must be non-empty, %s provided.',
@@ -34,7 +29,12 @@ public function postEvent(Event $requestData): void
3429
{
3530
try {
3631
$jobDataJson = json_encode($requestData->jsonSerialize(), JSON_THROW_ON_ERROR);
37-
$request = new Request('POST', sprintf('events/%s', $requestData->getEventType()), [], $jobDataJson);
32+
$request = new Request(
33+
'POST',
34+
sprintf('events/%s', $requestData->getEventType()),
35+
['Content-type' => 'application/json'],
36+
$jobDataJson
37+
);
3838
} catch (JsonException $e) {
3939
throw new ClientException('Invalid job data: ' . $e->getMessage(), $e->getCode(), $e);
4040
}

0 commit comments

Comments
 (0)