DGS-443 Stop reporting every import failure as a large-country timeout - #186
Open
TLabutis wants to merge 1 commit into
Open
DGS-443 Stop reporting every import failure as a large-country timeout#186TLabutis wants to merge 1 commit into
TLabutis wants to merge 1 commit into
Conversation
The parcel shop import error handler treated HTTP 500 the same as a timeout, so any server-side fatal opened the "Automatic Update Required" modal claiming the country has too many pick-up points. Merchants then ran the suggested cron command and hit the same fatal from the CLI, with no error ever surfacing in the UI. Only status 'timeout' and HTTP 504 now open that modal; a 500 shows a real error message pointing at the DPD logs page. The modal copy no longer asserts the country is large, and the cron command it prints uses --country=<iso> for the selected country instead of --all, matching what AdminDPDBalticsAjaxController already returns in cron_command. The underlying LT import fatal (Unirest\Request called statically) lives in invertus/dpdbaltics-api and is not addressed here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two symptoms were reported on the pick-up point import, and they are one bug plus one misdiagnosis.
Lithuania is not a large country.
Config::COUNTRIES_SKIP_OPENING_HOURSlists onlyPL. The message was wrong, and because it was wrong the merchant ran the suggested cron command and hit the same underlying fatal from the CLI, still with no real error shown anywhere.The cause is in
views/js/admin/import/import_parcels.js:Every HTTP 500 - any PHP fatal, any exception - was funnelled into the large-country modal. The server side was already correct:
AdminDPDBalticsAjaxController::handleImportError()inspects the message fortimeout/execution time/Maximum executionbefore returningrequires_cron, andbuildCronRequiredResponse()builds the command with--country=<iso>. Only the client-side fallback path was wrong.Changes
import_parcels.js- onlystatus === 'timeout'and HTTP 504 open the cron modal. HTTP 500 now shows a real error message pointing at the DPD logs page. The other branches are unchanged.import_parcels.js- newbuildCronCommand()prints--country=<iso>for the country the merchant actually selected, falling back to--allonly when no ISO is available. Previously the fallback path always suggested--all, which updates every country.AdminDPDBalticsImportExportController.php- passescountryIsoand a translatableserverErrorMessageto the JS.countryId(a PrestaShop ID) was already there but is not usable for--country.importing-parcels-popup.tpl- the modal now describes a timeout instead of asserting the country is large. TitleUpdate timed out; body explains the connection closed before the update finished and that the CLI has no time limit.CHANGELOG.md-## [3.3.2]entries.Still open - not fixed here
The actual LT 500 is
Non-static method Unirest\Request::timeout() cannot be called statically. The module shipsapimatic/unirest-php2.3.0, whoseRequestis static, but a v4 copy wins the autoloader on some setups, and"prepend-autoloader": falseincomposer.jsonmakes that ordering possible. The static calls live ininvertus/dpdbaltics-api(src/Api/ApiRequest.php), not in this repo, so the fix belongs there - either by scoping the vendor namespace or by moving to the instance API. Flippingprepend-autoloadertotruewould make DPD's Symfony 3.4 win over PrestaShop core's and is not an option.After this PR that failure is at least reported honestly instead of being disguised as a capacity limit.
Testing
node --check views/js/admin/import/import_parcels.jsphp -l controllers/admin/AdminDPDBalticsImportExportController.phpCountry::getIsoById()resolves - the controller is in the global namespace, same call pattern asAdminDPDBalticsAjaxController.php:52. Returnsfalsefor an unset selector, which falls back to--all.Manual check on the import page: a forced 500 shows the server error message; a request exceeding the 360s jQuery timeout shows the cron modal with
--country=LT.