fix: make bpn response failures diagnosable
This commit is contained in:
+73
-24
@@ -602,10 +602,12 @@ class ApiClient
|
||||
'port' => $this->selectedPort,
|
||||
]);
|
||||
$this->send($socket, $body);
|
||||
$response = $this->receive($socket);
|
||||
$this->disconnect($socket);
|
||||
|
||||
$responseXml = substr($response, 10);
|
||||
try {
|
||||
$responseXml = $this->receive($socket);
|
||||
} finally {
|
||||
$this->disconnect($socket);
|
||||
}
|
||||
|
||||
if (true === $debug || true === $this->config['debug']) {
|
||||
$this->dumpXmlToFile('response', $requestId, $responseXml);
|
||||
@@ -714,18 +716,12 @@ class ApiClient
|
||||
]);
|
||||
|
||||
$this->send($socket, $body);
|
||||
$response = $this->receive($socket);
|
||||
$this->disconnect($socket);
|
||||
|
||||
// message length (10 bytes) is prepended to actual message
|
||||
$xml = substr($response, 10);
|
||||
|
||||
if ('' === $xml) {
|
||||
$this->logger->error('Empty response body received from API (header-only response)', [
|
||||
'request_id' => $requestId,
|
||||
'raw_length' => strlen($response),
|
||||
]);
|
||||
throw new ApiClientException('Empty response body received from API');
|
||||
try {
|
||||
// the length header is consumed and verified by receive(), this is the payload
|
||||
$xml = $this->receive($socket);
|
||||
} finally {
|
||||
$this->disconnect($socket);
|
||||
}
|
||||
|
||||
if (true === $debug || true === $this->config['debug']) {
|
||||
@@ -735,14 +731,19 @@ class ApiClient
|
||||
try {
|
||||
return $this->responseParser->parseXmlString($type, $xml, $additionalArgs);
|
||||
} catch (ResponseParserException $e) {
|
||||
// Keep the evidence even outside debug mode: without the raw response a parse
|
||||
// failure is not diagnosable after the fact. app:cleanup:xml-dumps prunes it.
|
||||
$this->dumpXmlToFile('response', $requestId, $xml);
|
||||
|
||||
$this->logger->error('Unable to parse response received from API', [
|
||||
'request_id' => $requestId,
|
||||
'type' => $type,
|
||||
'response_length' => strlen($xml),
|
||||
'error_message' => $e->getMessage(),
|
||||
]);
|
||||
|
||||
throw new ApiClientException('Unable to parse response received from API: '.$e->getMessage(), 0, $e);
|
||||
}
|
||||
|
||||
$this->logger->error('Unexpected response received from API', [
|
||||
'request_id' => $requestId,
|
||||
]);
|
||||
|
||||
throw new ApiClientException('Unexpected response received from API');
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -773,10 +774,12 @@ class ApiClient
|
||||
]);
|
||||
|
||||
$this->send($socket, $body);
|
||||
$response = $this->receive($socket);
|
||||
$this->disconnect($socket);
|
||||
|
||||
$xml = substr($response, 10);
|
||||
try {
|
||||
$xml = $this->receive($socket);
|
||||
} finally {
|
||||
$this->disconnect($socket);
|
||||
}
|
||||
|
||||
if (true === $this->config['debug']) {
|
||||
$this->dumpXmlToFile('response', $requestId, $xml);
|
||||
@@ -898,8 +901,14 @@ class ApiClient
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads a single response message and returns its payload. The protocol prepends the
|
||||
* payload length as a 10 byte header, so the announced length is compared against what
|
||||
* actually arrived: a peer closing mid-stream would otherwise yield a silently
|
||||
* truncated body that only surfaces later as an unexplained parse failure.
|
||||
*
|
||||
* @param resource $socket
|
||||
*
|
||||
* @throws ApiClientException
|
||||
* @throws TimeoutException
|
||||
* @throws ImmediateConnectionCloseException
|
||||
*/
|
||||
@@ -963,7 +972,47 @@ class ApiClient
|
||||
throw new ImmediateConnectionCloseException('Server closed connection without sending data');
|
||||
}
|
||||
|
||||
return $response;
|
||||
return $this->extractPayload($response);
|
||||
}
|
||||
|
||||
/**
|
||||
* @throws ApiClientException
|
||||
*/
|
||||
private function extractPayload(string $response): string
|
||||
{
|
||||
if (10 > strlen($response)) {
|
||||
$this->logger->error('Incomplete response header received from API', [
|
||||
'bytes_received' => strlen($response),
|
||||
'port' => $this->selectedPort,
|
||||
]);
|
||||
|
||||
throw new ApiClientException(sprintf('Incomplete response header, got %d of 10 bytes', strlen($response)));
|
||||
}
|
||||
|
||||
$header = substr($response, 0, 10);
|
||||
$announcedLength = (int) trim($header);
|
||||
$payload = substr($response, 10);
|
||||
|
||||
if (1 > $announcedLength) {
|
||||
$this->logger->error('Empty response body announced by API', [
|
||||
'header' => trim($header),
|
||||
'port' => $this->selectedPort,
|
||||
]);
|
||||
|
||||
throw new ApiClientException(sprintf('Response announced an empty body (header "%s")', trim($header)));
|
||||
}
|
||||
|
||||
if (strlen($payload) !== $announcedLength) {
|
||||
$this->logger->error('Truncated response received from API', [
|
||||
'bytes_received' => strlen($payload),
|
||||
'announced_length' => $announcedLength,
|
||||
'port' => $this->selectedPort,
|
||||
]);
|
||||
|
||||
throw new ApiClientException(sprintf('Truncated response, got %d of %d announced bytes', strlen($payload), $announcedLength));
|
||||
}
|
||||
|
||||
return $payload;
|
||||
}
|
||||
|
||||
/** @param resource $socket */
|
||||
|
||||
@@ -20,4 +20,38 @@ final class XmlCrawlerFactory
|
||||
|
||||
return $crawler;
|
||||
}
|
||||
|
||||
/**
|
||||
* Explains why create() returned an empty Crawler. addXmlContent() discards the
|
||||
* libxml errors, so an empty, a malformed and a truncated payload are
|
||||
* indistinguishable afterwards; this re-parses to recover the reason. Only worth
|
||||
* calling on the failure path.
|
||||
*/
|
||||
public static function diagnose(string $xml): string
|
||||
{
|
||||
if ('' === trim($xml)) {
|
||||
return sprintf('empty response (%d bytes)', strlen($xml));
|
||||
}
|
||||
|
||||
$previousUseErrors = libxml_use_internal_errors(true);
|
||||
libxml_clear_errors();
|
||||
|
||||
try {
|
||||
simplexml_load_string($xml);
|
||||
|
||||
$messages = [];
|
||||
foreach (libxml_get_errors() as $error) {
|
||||
$messages[] = sprintf('%s (line %d, column %d)', trim($error->message), $error->line, $error->column);
|
||||
}
|
||||
|
||||
if ([] === $messages) {
|
||||
return sprintf('unknown XML error (%d bytes)', strlen($xml));
|
||||
}
|
||||
|
||||
return sprintf('%s (%d bytes)', implode('; ', array_unique($messages)), strlen($xml));
|
||||
} finally {
|
||||
libxml_clear_errors();
|
||||
libxml_use_internal_errors($previousUseErrors);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ class ApiResponseParser extends AbstractParser
|
||||
$crawler = XmlCrawlerFactory::create($xml);
|
||||
|
||||
if (0 === $crawler->count()) {
|
||||
throw new ResponseParserException('Empty response received from server');
|
||||
throw new ResponseParserException('Unable to parse XML response: '.XmlCrawlerFactory::diagnose($xml));
|
||||
}
|
||||
|
||||
// Override type unless present in XML to catch error responses
|
||||
@@ -56,7 +56,8 @@ class ApiResponseParser extends AbstractParser
|
||||
case 'Vorgangdruck':
|
||||
return (new DocumentsParser())->parseConfirmation($resultNode);
|
||||
}
|
||||
break;
|
||||
|
||||
throw new ResponseParserException(sprintf('Unrecognised customer data subtype "%s"', $subType));
|
||||
case ApiClient::TYPE_BASE_DATA_COUNTRIES:
|
||||
return (new CountriesParser())->parse($resultNode);
|
||||
case ApiClient::TYPE_MUTABLE_DATA:
|
||||
@@ -92,6 +93,6 @@ class ApiResponseParser extends AbstractParser
|
||||
throw new ResponseParserException('Invalid XML response structure: '.$e->getMessage(), 0, $e);
|
||||
}
|
||||
|
||||
throw new ResponseParserException('Unable to parse XML response');
|
||||
throw new ResponseParserException(sprintf('Unrecognised BusProNet response type "%s"', $responseType));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ namespace App\Tests\BusProNet;
|
||||
|
||||
use App\BusProNet\ApiClient;
|
||||
use App\BusProNet\DataProcessor\BookingDataProcessor;
|
||||
use App\BusProNet\Exception\ApiClientException;
|
||||
use App\BusProNet\Exception\ImmediateConnectionCloseException;
|
||||
use App\BusProNet\XmlParser\ApiResponseParser;
|
||||
use App\Service\RequestIdGenerator;
|
||||
@@ -70,7 +71,7 @@ class ApiClientReceiveTest extends TestCase
|
||||
$this->invokeReceive($stream);
|
||||
}
|
||||
|
||||
public function testNonEmptyResponseReturnsData(): void
|
||||
public function testNonEmptyResponseReturnsPayloadWithoutTheLengthHeader(): void
|
||||
{
|
||||
$stream = fopen('php://memory', 'r+');
|
||||
fwrite($stream, '0000000005Hello');
|
||||
@@ -80,7 +81,56 @@ class ApiClientReceiveTest extends TestCase
|
||||
|
||||
$result = $this->invokeReceive($stream);
|
||||
|
||||
self::assertSame('0000000005Hello', $result);
|
||||
self::assertSame('Hello', $result);
|
||||
}
|
||||
|
||||
/**
|
||||
* A peer closing mid-stream used to yield a silently shortened payload, which only
|
||||
* surfaced later as an unexplained parse failure. The announced length catches it here.
|
||||
*/
|
||||
public function testTruncatedResponseThrows(): void
|
||||
{
|
||||
$stream = fopen('php://memory', 'r+');
|
||||
fwrite($stream, '0000000050Hello');
|
||||
rewind($stream);
|
||||
|
||||
$this->setOperationStartTime();
|
||||
$this->setSelectedPort(9000);
|
||||
|
||||
$this->expectException(ApiClientException::class);
|
||||
$this->expectExceptionMessage('Truncated response, got 5 of 50 announced bytes');
|
||||
|
||||
$this->invokeReceive($stream);
|
||||
}
|
||||
|
||||
public function testResponseShorterThanTheLengthHeaderThrows(): void
|
||||
{
|
||||
$stream = fopen('php://memory', 'r+');
|
||||
fwrite($stream, '00000');
|
||||
rewind($stream);
|
||||
|
||||
$this->setOperationStartTime();
|
||||
$this->setSelectedPort(9000);
|
||||
|
||||
$this->expectException(ApiClientException::class);
|
||||
$this->expectExceptionMessage('Incomplete response header, got 5 of 10 bytes');
|
||||
|
||||
$this->invokeReceive($stream);
|
||||
}
|
||||
|
||||
public function testHeaderOnlyResponseThrows(): void
|
||||
{
|
||||
$stream = fopen('php://memory', 'r+');
|
||||
fwrite($stream, '0000000000');
|
||||
rewind($stream);
|
||||
|
||||
$this->setOperationStartTime();
|
||||
$this->setSelectedPort(9000);
|
||||
|
||||
$this->expectException(ApiClientException::class);
|
||||
$this->expectExceptionMessage('Response announced an empty body (header "0000000000")');
|
||||
|
||||
$this->invokeReceive($stream);
|
||||
}
|
||||
|
||||
private function invokeReceive($socket): string
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\BusProNet\XmlParser;
|
||||
|
||||
use App\BusProNet\ApiClient;
|
||||
use App\BusProNet\Exception\ResponseParserException;
|
||||
use App\BusProNet\XmlParser\ApiResponseParser;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* A broken response used to collapse into either "Empty response received from server" —
|
||||
* which is wrong for a malformed body — or an opaque "Unable to parse XML response".
|
||||
* Each shape must now name itself, otherwise a production parse failure is not
|
||||
* diagnosable after the fact.
|
||||
*/
|
||||
class ApiResponseParserFailureTest extends TestCase
|
||||
{
|
||||
public function testEmptyResponse(): void
|
||||
{
|
||||
$this->expectException(ResponseParserException::class);
|
||||
$this->expectExceptionMessage('empty response (0 bytes)');
|
||||
|
||||
(new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, '');
|
||||
}
|
||||
|
||||
public function testTruncatedResponseNamesTheLibxmlReason(): void
|
||||
{
|
||||
$xml = '<?xml version="1.0" encoding="utf-8"?><ergebnis><satz typ="HINWEIS"></satz><hinweis';
|
||||
|
||||
try {
|
||||
(new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml);
|
||||
$this->fail('Expected a ResponseParserException');
|
||||
} catch (ResponseParserException $e) {
|
||||
self::assertStringContainsString('('.strlen($xml).' bytes)', $e->getMessage());
|
||||
self::assertStringContainsString('line 1', $e->getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
public function testWellFormedResponseOfAnUnknownType(): void
|
||||
{
|
||||
$xml = '<?xml version="1.0" encoding="utf-8"?><ergebnis><satz typ="IRGENDWAS"></satz></ergebnis>';
|
||||
|
||||
$this->expectException(ResponseParserException::class);
|
||||
$this->expectExceptionMessage('Unrecognised BusProNet response type "IRGENDWAS"');
|
||||
|
||||
(new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml);
|
||||
}
|
||||
|
||||
public function testUnknownCustomerDataSubtype(): void
|
||||
{
|
||||
$xml = '<?xml version="1.0" encoding="utf-8"?><ergebnis><satz typ="KUNDENKONTO"></satz><art>Irgendwas</art></ergebnis>';
|
||||
|
||||
$this->expectException(ResponseParserException::class);
|
||||
$this->expectExceptionMessage('Unrecognised customer data subtype "Irgendwas"');
|
||||
|
||||
(new ApiResponseParser())->parseXmlString(ApiClient::TYPE_CUSTOMER_DATA, $xml);
|
||||
}
|
||||
|
||||
public function testDiagnosingDoesNotLeakLibxmlErrorState(): void
|
||||
{
|
||||
$previous = libxml_use_internal_errors(false);
|
||||
|
||||
try {
|
||||
try {
|
||||
(new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, '<ergebnis');
|
||||
} catch (ResponseParserException) {
|
||||
}
|
||||
|
||||
self::assertFalse(libxml_use_internal_errors(false));
|
||||
self::assertSame([], libxml_get_errors());
|
||||
} finally {
|
||||
libxml_use_internal_errors($previous);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user