Skip to content

Commit 1327f54

Browse files
rostyslav-hymonrostyslav-hymon
committed
MC-38247: Failure of connections to UPS XML/USPS/DHL API will cause the checkout to break
1 parent c644b3d commit 1327f54

File tree

7 files changed

+213
-19
lines changed

7 files changed

+213
-19
lines changed

‎app/code/Magento/Dhl/Model/Carrier.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,23 @@
77
namespace Magento\Dhl\Model;
88

99
use Magento\Catalog\Model\Product\Type;
10+
use Magento\Dhl\Model\Validator\XmlValidator;
1011
use Magento\Framework\App\ObjectManager;
1112
use Magento\Framework\App\ProductMetadataInterface;
1213
use Magento\Framework\Async\CallbackDeferred;
14+
use Magento\Framework\HTTP\AsyncClient\HttpException;
1315
use Magento\Framework\HTTP\AsyncClient\HttpResponseDeferredInterface;
1416
use Magento\Framework\HTTP\AsyncClient\Request;
1517
use Magento\Framework\HTTP\AsyncClientInterface;
1618
use Magento\Framework\Module\Dir;
17-
use Magento\Sales\Exception\DocumentValidationException;
18-
use Magento\Sales\Model\Order\Shipment;
19+
use Magento\Framework\Xml\Security;
1920
use Magento\Quote\Model\Quote\Address\RateRequest;
2021
use Magento\Quote\Model\Quote\Address\RateResult\Error;
22+
use Magento\Sales\Exception\DocumentValidationException;
23+
use Magento\Sales\Model\Order\Shipment;
2124
use Magento\Shipping\Model\Carrier\AbstractCarrier;
2225
use Magento\Shipping\Model\Rate\Result;
2326
use Magento\Shipping\Model\Rate\Result\ProxyDeferredFactory;
24-
use Magento\Framework\Xml\Security;
25-
use Magento\Dhl\Model\Validator\XmlValidator;
2627

2728
/**
2829
* DHL International (API v1.4)
@@ -1064,8 +1065,15 @@ protected function _getQuotes()
10641065
function () use ($deferredResponses, $responseBodies) {
10651066
//Loading rates not found in cache
10661067
foreach ($deferredResponses as $deferredResponseData) {
1068+
$responseResult = null;
1069+
try {
1070+
$responseResult = $deferredResponseData['deferred']->get();
1071+
} catch (HttpException $exception) {
1072+
$this->_logger->critical($exception);
1073+
}
1074+
$responseBody = $responseResult ? $responseResult->getBody() : '';
10671075
$responseBodies[] = [
1068-
'body' => $deferredResponseData['deferred']->get()->getBody(),
1076+
'body' => $responseBody,
10691077
'date' => $deferredResponseData['date'],
10701078
'request' => $deferredResponseData['request'],
10711079
'from_cache' => false
@@ -2028,7 +2036,8 @@ protected function _checkDomesticStatus($origCountryCode, $destCountryCode)
20282036
$isDomestic = (string)$this->getCountryParams($destCountryCode)->getData('domestic');
20292037

20302038
if (($origCountry == $destCountry && $isDomestic)
2031-
|| ($this->_carrierHelper->isCountryInEU($origCountryCode)
2039+
|| (
2040+
$this->_carrierHelper->isCountryInEU($origCountryCode)
20322041
&& $this->_carrierHelper->isCountryInEU($destCountryCode)
20332042
)
20342043
) {

‎app/code/Magento/Ups/Model/Carrier.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Magento\Framework\Async\CallbackDeferred;
1818
use Magento\Framework\DataObject;
1919
use Magento\Framework\Exception\LocalizedException;
20+
use Magento\Framework\HTTP\AsyncClient\HttpException;
2021
use Magento\Framework\HTTP\AsyncClient\HttpResponseDeferredInterface;
2122
use Magento\Framework\HTTP\AsyncClient\Request;
2223
use Magento\Framework\HTTP\AsyncClientInterface;
@@ -33,14 +34,14 @@
3334
use Magento\Shipping\Model\Rate\Result;
3435
use Magento\Shipping\Model\Rate\Result\ProxyDeferredFactory;
3536
use Magento\Shipping\Model\Rate\ResultFactory as RateFactory;
37+
use Magento\Shipping\Model\Shipment\Request as Shipment;
3638
use Magento\Shipping\Model\Simplexml\Element;
3739
use Magento\Shipping\Model\Simplexml\ElementFactory;
3840
use Magento\Shipping\Model\Tracking\Result\ErrorFactory as TrackErrorFactory;
3941
use Magento\Shipping\Model\Tracking\Result\StatusFactory as TrackStatusFactory;
4042
use Magento\Shipping\Model\Tracking\ResultFactory as TrackFactory;
4143
use Magento\Store\Model\ScopeInterface;
4244
use Magento\Ups\Helper\Config;
43-
use Magento\Shipping\Model\Shipment\Request as Shipment;
4445
use Psr\Log\LoggerInterface;
4546
use RuntimeException;
4647
use Throwable;
@@ -811,10 +812,15 @@ protected function _getXmlQuotes()
811812
[
812813
'deferred' => new CallbackDeferred(
813814
function () use ($httpResponse) {
814-
if ($httpResponse->get()->getStatusCode() >= 400) {
815-
$xmlResponse = '';
816-
} else {
817-
$xmlResponse = $httpResponse->get()->getBody();
815+
$responseResult = null;
816+
$xmlResponse = '';
817+
try {
818+
$responseResult = $httpResponse->get();
819+
} catch (HttpException $exception) {
820+
$this->_logger->critical($exception);
821+
}
822+
if ($responseResult) {
823+
$xmlResponse = $responseResult->getStatusCode() >= 400 ? '' : $responseResult->getBody();
818824
}
819825

820826
return $this->_parseXmlResponse($xmlResponse);

‎app/code/Magento/Usps/Model/Carrier.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
use Magento\Framework\App\ObjectManager;
1010
use Magento\Framework\Async\CallbackDeferred;
11+
use Magento\Framework\DataObject;
12+
use Magento\Framework\HTTP\AsyncClient\HttpException;
1113
use Magento\Framework\HTTP\AsyncClient\Request;
1214
use Magento\Framework\HTTP\AsyncClientInterface;
1315
use Magento\Framework\Xml\Security;
@@ -25,19 +27,19 @@
2527
*/
2628
class Carrier extends AbstractCarrierOnline implements \Magento\Shipping\Model\Carrier\CarrierInterface
2729
{
28-
/** @deprecated */
30+
/** @deprecated Redundant dependency */
2931
const CONTAINER_VARIABLE = 'VARIABLE';
3032

31-
/** @deprecated */
33+
/** @deprecated Redundant dependency */
3234
const CONTAINER_FLAT_RATE_BOX = 'FLAT RATE BOX';
3335

34-
/** @deprecated */
36+
/** @deprecated Redundant dependency */
3537
const CONTAINER_FLAT_RATE_ENVELOPE = 'FLAT RATE ENVELOPE';
3638

37-
/** @deprecated */
39+
/** @deprecated Redundant dependency */
3840
const CONTAINER_RECTANGULAR = 'RECTANGULAR';
3941

40-
/** @deprecated */
42+
/** @deprecated Redundant dependency */
4143
const CONTAINER_NONRECTANGULAR = 'NONRECTANGULAR';
4244

4345
/**
@@ -149,6 +151,11 @@ class Carrier extends AbstractCarrierOnline implements \Magento\Shipping\Model\C
149151
*/
150152
private $proxyDeferredFactory;
151153

154+
/**
155+
* @var DataObject
156+
*/
157+
private $_rawTrackRequest;
158+
152159
/**
153160
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
154161
* @param \Magento\Quote\Model\Quote\Address\RateResult\ErrorFactory $rateErrorFactory
@@ -560,7 +567,13 @@ protected function _getXmlQuotes()
560567
[
561568
'deferred' => new CallbackDeferred(
562569
function () use ($deferredResponse, $request, $debugData) {
563-
$responseBody = $deferredResponse->get()->getBody();
570+
$responseResult = null;
571+
try {
572+
$responseResult = $deferredResponse->get();
573+
} catch (HttpException $exception) {
574+
$this->_logger->critical($exception);
575+
}
576+
$responseBody = $responseResult ? $responseResult->getBody() : '';
564577
$debugData['result'] = $responseBody;
565578
$this->_setCachedQuotes($request, $responseBody);
566579
$this->_debug($debugData);

‎dev/tests/integration/framework/Magento/TestFramework/HTTP/AsyncClientInterfaceMock.php

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ class AsyncClientInterfaceMock implements AsyncClientInterface
3838
*/
3939
private $requests = [];
4040

41+
/**
42+
* @var HttpResponseDeferredInterface
43+
*/
44+
private $mockDeferredResponse;
45+
4146
/**
4247
* AsyncClientInterfaceMock constructor.
4348
* @param GuzzleAsyncClient $client
@@ -89,15 +94,28 @@ public function clearRequests()
8994
$this->lastRequest = null;
9095
}
9196

97+
/**
98+
* Next responses will be as given.
99+
*
100+
* @param HttpResponseDeferredInterface $mockDeferredResponse
101+
* @return self
102+
*/
103+
public function setDeferredResponseMock(HttpResponseDeferredInterface $mockDeferredResponse): self
104+
{
105+
$this->mockDeferredResponse = $mockDeferredResponse;
106+
107+
return $this;
108+
}
109+
92110
/**
93111
* @inheritDoc
94112
*/
95113
public function request(Request $request): HttpResponseDeferredInterface
96114
{
97115
$this->lastRequest = $request;
98116
$this->requests[] = $request;
99-
if ($mockResponse = array_shift($this->mockResponses)) {
100-
return new MockDeferredResponse($mockResponse);
117+
if ($mockResponse = $this->mockDeferredResponse ?? array_shift($this->mockResponses)) {
118+
return $this->mockDeferredResponse ?? new MockDeferredResponse($mockResponse);
101119
}
102120

103121
return $this->client->request($request);

‎dev/tests/integration/testsuite/Magento/Dhl/Model/CarrierTest.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99

1010
use Magento\Framework\App\Config\ReinitableConfigInterface;
1111
use Magento\Framework\DataObject;
12+
use Magento\Framework\HTTP\AsyncClient\HttpException;
13+
use Magento\Framework\HTTP\AsyncClient\HttpResponseDeferredInterface;
1214
use Magento\Framework\HTTP\AsyncClient\Response;
1315
use Magento\Framework\HTTP\AsyncClientInterface;
1416
use Magento\Framework\Simplexml\Element;
1517
use Magento\Quote\Model\Quote\Address\RateRequest;
18+
use Magento\Quote\Model\Quote\Address\RateResult\Error;
1619
use Magento\Shipping\Model\Shipment\Request;
1720
use Magento\Shipping\Model\Tracking\Result\Status;
1821
use Magento\Store\Model\ScopeInterface;
@@ -475,6 +478,33 @@ public function testCollectRatesWithoutDimensions(?string $size, ?string $height
475478
$this->config->reinit();
476479
}
477480

481+
/**
482+
* Test get carriers rates if has HttpException.
483+
*
484+
* @magentoConfigFixture default_store carriers/dhl/active 1
485+
*/
486+
public function testGetRatesWithHttpException(): void
487+
{
488+
$this->setDhlConfig(['showmethod' => 1]);
489+
$requestData = $this->getRequestData();
490+
$deferredResponse = $this->getMockBuilder(HttpResponseDeferredInterface::class)
491+
->onlyMethods(['get'])
492+
->getMockForAbstractClass();
493+
$exception = new HttpException('Exception message');
494+
$deferredResponse->method('get')->willThrowException($exception);
495+
$this->httpClient->setDeferredResponseMock($deferredResponse);
496+
/** @var RateRequest $request */
497+
$request = Bootstrap::getObjectManager()->create(RateRequest::class, $requestData);
498+
$this->dhlCarrier = Bootstrap::getObjectManager()->create(Carrier::class);
499+
$resultRate = $this->dhlCarrier->collectRates($request)->getAllRates()[0];
500+
$error = Bootstrap::getObjectManager()->get(Error::class);
501+
$error->setCarrier('dhl');
502+
$error->setCarrierTitle($this->dhlCarrier->getConfigData('title'));
503+
$error->setErrorMessage($this->dhlCarrier->getConfigData('specificerrmsg'));
504+
505+
$this->assertEquals($error, $resultRate);
506+
}
507+
478508
/**
479509
* @return array
480510
*/

‎dev/tests/integration/testsuite/Magento/Ups/Model/CarrierTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@
99

1010
use Magento\Framework\App\Config\ReinitableConfigInterface;
1111
use Magento\Framework\DataObject;
12+
use Magento\Framework\HTTP\AsyncClient\HttpException;
13+
use Magento\Framework\HTTP\AsyncClient\HttpResponseDeferredInterface;
1214
use Magento\Framework\HTTP\AsyncClient\Response;
1315
use Magento\Framework\HTTP\AsyncClientInterface;
1416
use Magento\Quote\Model\Quote\Address\RateRequest;
17+
use Magento\Quote\Model\Quote\Address\RateResult\Error;
1518
use Magento\TestFramework\Helper\Bootstrap;
1619
use Magento\Quote\Model\Quote\Address\RateRequestFactory;
1720
use Magento\TestFramework\HTTP\AsyncClientInterfaceMock;
@@ -290,6 +293,50 @@ public function testRequestToShipment(): void
290293
$this->httpClient->clearRequests();
291294
}
292295

296+
/**
297+
* Test get carriers rates if has HttpException.
298+
*
299+
* @magentoConfigFixture default_store shipping/origin/country_id GB
300+
* @magentoConfigFixture default_store carriers/ups/type UPS_XML
301+
* @magentoConfigFixture default_store carriers/ups/active 1
302+
* @magentoConfigFixture default_store carriers/ups/shipper_number 12345
303+
* @magentoConfigFixture default_store carriers/ups/origin_shipment Shipments Originating in the European Union
304+
* @magentoConfigFixture default_store carriers/ups/username user
305+
* @magentoConfigFixture default_store carriers/ups/password pass
306+
* @magentoConfigFixture default_store carriers/ups/access_license_number acn
307+
* @magentoConfigFixture default_store currency/options/allow GBP,USD,EUR
308+
* @magentoConfigFixture default_store currency/options/base GBP
309+
*/
310+
public function testGetRatesWithHttpException(): void
311+
{
312+
$deferredResponse = $this->getMockBuilder(HttpResponseDeferredInterface::class)
313+
->onlyMethods(['get'])
314+
->getMockForAbstractClass();
315+
$exception = new HttpException('Exception message');
316+
$deferredResponse->method('get')->willThrowException($exception);
317+
$this->httpClient->setDeferredResponseMock($deferredResponse);
318+
$request = Bootstrap::getObjectManager()->create(
319+
RateRequest::class,
320+
[
321+
'data' => [
322+
'dest_country' => 'GB',
323+
'dest_postal' => '01105',
324+
'product' => '11',
325+
'action' => 'Rate',
326+
'unit_measure' => 'KGS',
327+
'base_currency' => new DataObject(['code' => 'GBP'])
328+
]
329+
]
330+
);
331+
$resultRate = $this->carrier->collectRates($request)->getAllRates()[0];
332+
$error = Bootstrap::getObjectManager()->get(Error::class);
333+
$error->setCarrier('ups');
334+
$error->setCarrierTitle($this->carrier->getConfigData('title'));
335+
$error->setErrorMessage($this->carrier->getConfigData('specificerrmsg'));
336+
337+
$this->assertEquals($error, $resultRate);
338+
}
339+
293340
/**
294341
* Extracts shipment request.
295342
*

‎dev/tests/integration/testsuite/Magento/Usps/Model/CarrierTest.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
use Magento\TestFramework\Helper\Bootstrap;
1414
use Magento\TestFramework\HTTP\AsyncClientInterfaceMock;
1515
use PHPUnit\Framework\TestCase;
16+
use Magento\Framework\HTTP\AsyncClient\HttpResponseDeferredInterface;
17+
use Magento\Framework\HTTP\AsyncClient\HttpException;
18+
use Magento\Quote\Model\Quote\Address\RateResult\Error;
1619

1720
/**
1821
* Test for USPS integration.
@@ -176,4 +179,72 @@ public function testCollectUnavailableRates(): void
176179
$rates = $this->carrier->collectRates($request);
177180
$this->assertCount(5, $rates->getAllRates());
178181
}
182+
183+
/**
184+
* Test get carriers rates if has HttpException.
185+
*
186+
* @magentoConfigFixture default_store carriers/usps/allowed_methods 0_FCLE,0_FCL,0_FCP,1,2,3,4,6,7,13,16,17,22,23,25,27,28,33,34,35,36,37,42,43,53,55,56,57,61,INT_1,INT_2,INT_4,INT_6,INT_7,INT_8,INT_9,INT_10,INT_11,INT_12,INT_13,INT_14,INT_15,INT_16,INT_20,INT_26
187+
* @magentoConfigFixture default_store carriers/usps/showmethod 1
188+
* @magentoConfigFixture default_store carriers/usps/debug 1
189+
* @magentoConfigFixture default_store carriers/usps/userid test
190+
* @magentoConfigFixture default_store carriers/usps/mode 0
191+
* @magentoConfigFixture default_store carriers/usps/active 1
192+
* @magentoConfigFixture default_store shipping/origin/country_id US
193+
* @magentoConfigFixture default_store shipping/origin/postcode 90034
194+
* @magentoConfigFixture default_store carriers/usps/machinable true
195+
*/
196+
public function testGetRatesWithHttpException(): void
197+
{
198+
$deferredResponse = $this->getMockBuilder(HttpResponseDeferredInterface::class)
199+
->onlyMethods(['get'])
200+
->getMockForAbstractClass();
201+
$exception = new HttpException('Exception message');
202+
$deferredResponse->method('get')->willThrowException($exception);
203+
$this->httpClient->setDeferredResponseMock($deferredResponse);
204+
/** @var RateRequest $request */
205+
$request = Bootstrap::getObjectManager()->create(
206+
RateRequest::class,
207+
[
208+
'data' => [
209+
'dest_country_id' => 'US',
210+
'dest_region_code' => 'NY',
211+
'dest_street' => 'main st1',
212+
'dest_city' => 'New York',
213+
'dest_postcode' => '10029',
214+
'package_value' => '5',
215+
'package_value_with_discount' => '5',
216+
'package_weight' => '4.2657',
217+
'package_qty' => '1',
218+
'package_physical_value' => '5',
219+
'free_method_weight' => '5',
220+
'store_id' => '1',
221+
'website_id' => '1',
222+
'free_shipping' => '0',
223+
'limit_carrier' => 'null',
224+
'base_subtotal_incl_tax' => '5',
225+
'orig_country_id' => 'US',
226+
'country_id' => 'US',
227+
'region_id' => '12',
228+
'city' => 'Culver City',
229+
'postcode' => '90034',
230+
'usps_userid' => '213MAGEN6752',
231+
'usps_container' => 'VARIABLE',
232+
'usps_size' => 'REGULAR',
233+
'girth' => null,
234+
'height' => null,
235+
'length' => null,
236+
'width' => null,
237+
]
238+
]
239+
);
240+
241+
$rates = $this->carrier->collectRates($request);
242+
$resultRate = $rates->getAllRates()[0];
243+
$error = Bootstrap::getObjectManager()->get(Error::class);
244+
$error->setCarrier('usps');
245+
$error->setCarrierTitle($this->carrier->getConfigData('title'));
246+
$error->setErrorMessage($this->carrier->getConfigData('specificerrmsg'));
247+
248+
$this->assertEquals($error, $resultRate);
249+
}
179250
}

0 commit comments

Comments
 (0)