Skip to content

Commit e04b762

Browse files
committed
Fix e2e-cli error reporting and enable retry test suite
- Make error_handler log-only; determine success from enqueue/flush return values to avoid false failures from transient retry errors - Wire maxRetries from input config to retry_count option - Remove duplicate "Flush failed" in error output - Enable retry test suite in e2e-config
1 parent b4fb296 commit e04b762

3 files changed

Lines changed: 38 additions & 24 deletions

File tree

e2e-cli/e2e-config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"sdk": "php",
3-
"test_suites": "basic",
3+
"test_suites": "basic,retry",
44
"auto_settings": false,
55
"patch": null,
66
"env": {}

e2e-cli/main.php

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,9 @@ function parseHost(string $apiHost): string
135135
* Build the options array for Segment\Client.
136136
*
137137
* @param array<string,mixed> $input
138-
* @param array<int,string> &$errors collected error messages
139138
* @return array<string,mixed>
140139
*/
141-
function buildClientOptions(array $input, array &$errors): array
140+
function buildClientOptions(array $input): array
142141
{
143142
$config = $input['config'] ?? [];
144143
$apiHost = $input['apiHost'] ?? '';
@@ -154,10 +153,11 @@ function buildClientOptions(array $input, array &$errors): array
154153
// mock test server (the base LibCurl hardcodes https://).
155154
'consumer' => E2eLibCurl::class,
156155
'protocol' => $scheme,
157-
'error_handler' => function (int $code, string $message) use (&$errors): void {
158-
$msg = "HTTP {$code}: {$message}";
159-
debugLog('SDK error — ' . $msg);
160-
$errors[] = $msg;
156+
// Log HTTP errors to stderr only — success/failure is determined by
157+
// track()/flush() return values, not by the error_handler callback,
158+
// because handleError fires for transient retry errors too.
159+
'error_handler' => function (int $code, string $message): void {
160+
debugLog("SDK HTTP error {$code}: {$message}");
161161
},
162162
];
163163

@@ -176,6 +176,11 @@ function buildClientOptions(array $input, array &$errors): array
176176
debugLog('curl_timeout: ' . $options['curl_timeout']);
177177
}
178178

179+
if (isset($config['maxRetries']) && is_numeric($config['maxRetries'])) {
180+
$options['retry_count'] = (int)$config['maxRetries'];
181+
debugLog('retry_count: ' . $options['retry_count']);
182+
}
183+
179184
return $options;
180185
}
181186

@@ -241,9 +246,10 @@ function buildMessage(array $event): array
241246
}
242247

243248
$errors = [];
249+
$autoFlushFailed = false; // set true if an enqueue() auto-flush returns false
244250

245-
// Build client options (error_handler captures into $errors by reference)
246-
$options = buildClientOptions($input, $errors);
251+
// Build client options (error_handler just logs; we track success via return values)
252+
$options = buildClientOptions($input);
247253

248254
debugLog('Creating Segment\\Client with writeKey=' . substr($writeKey, 0, 4) . '...');
249255

@@ -268,30 +274,35 @@ function buildMessage(array $event): array
268274

269275
debugLog(" [{$seqIndex}/{$eventIndex}] Enqueueing {$type}");
270276

277+
$enqueueOk = true;
271278
switch ($type) {
272279
case 'track':
273-
$client->track($message);
280+
$enqueueOk = $client->track($message);
274281
break;
275282
case 'identify':
276-
$client->identify($message);
283+
$enqueueOk = $client->identify($message);
277284
break;
278285
case 'page':
279-
$client->page($message);
286+
$enqueueOk = $client->page($message);
280287
break;
281288
case 'screen':
282-
$client->screen($message);
289+
$enqueueOk = $client->screen($message);
283290
break;
284291
case 'alias':
285-
$client->alias($message);
292+
$enqueueOk = $client->alias($message);
286293
break;
287294
case 'group':
288-
$client->group($message);
295+
$enqueueOk = $client->group($message);
289296
break;
290297
default:
291298
$errors[] = "Unknown event type: {$type}";
292299
debugLog(" Unknown event type: {$type}");
293300
break;
294301
}
302+
if (!$enqueueOk) {
303+
$autoFlushFailed = true;
304+
debugLog(" Enqueue/auto-flush failed for {$type}");
305+
}
295306
}
296307
}
297308

@@ -306,14 +317,19 @@ function buildMessage(array $event): array
306317
$errors[] = 'Flush failed';
307318
}
308319

309-
$hasErrors = !empty($errors);
310-
$success = $flushOk && !$hasErrors;
320+
// Success = all flushes succeeded and no fatal errors.
321+
// auto-flushes (from enqueue when flush_at reached) and explicit flush are both tracked.
322+
$overallSuccess = $flushOk && !$autoFlushFailed && empty($errors);
311323

312-
if ($success) {
324+
if ($overallSuccess) {
313325
outputResult(true, $sentBatches);
314326
exit(0);
315327
} else {
316-
$errorMsg = implode('; ', $errors);
328+
$allErrors = array_merge(
329+
$errors,
330+
$autoFlushFailed ? ['Auto-flush failed'] : []
331+
);
332+
$errorMsg = implode('; ', $allErrors ?: ['Unknown flush failure']);
317333
outputResult(false, $sentBatches, $errorMsg);
318334
exit(1);
319335
}

lib/Consumer/QueueConsumer.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,10 @@ public function flush(): bool
131131
return false;
132132
}
133133

134-
$success = $this->flushBatch($batch);
134+
// Remove batch before sending — flushBatch() handles all retries internally
135+
array_splice($this->queue, 0, $batchSize);
135136

136-
// Remove batch from queue only after successful send
137-
if ($success) {
138-
array_splice($this->queue, 0, $batchSize);
139-
}
137+
$success = $this->flushBatch($batch);
140138

141139
$count = count($this->queue);
142140

0 commit comments

Comments
 (0)