Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch down network exception and non-object responses in fetch #2492

Merged
merged 5 commits into from
Sep 27, 2023

Conversation

jms-pantheon
Copy link
Collaborator

Catching of exceptions arising from two states.

1) Network is completely down.

This would previously give an uncaught exception when doing any operation from Terminus.

$ terminus site:info jms-vanilla-wp
PHP Fatal error:  Uncaught Error: Call to a member function getBody() on null in phar:///usr/local/Cellar/terminus/3.2.1/bin/terminus/src/Request/Request.php:239
Stack trace:
#0 /Users/jms/.terminus/terminus-dependencies-27a446d993/vendor/guzzlehttp/guzzle/src/RetryMiddleware.php(101): Pantheon\Terminus\Request\Request->Pantheon\Terminus\Request\{closure}(5, Object(GuzzleHttp\Psr7\Request), NULL, Object(GuzzleHttp\Exception\ConnectException))

That is now caught and handled

$ terminus site:info jms-vanilla-wp
 [error]  HTTP request GET https://terminus.pantheon.io/api/site-names/jms-vanilla-wp has failed with error Connection refused for URI https://terminus.pantheon.io/api/site-names/jms-vanilla-wp.
 [error]  Could not locate a site your user may access identified by jms-vanilla-wp: HTTP request has failed with error "Maximum retry attempts reached".

2) Invalid network responses.

This I couldn't reproduce, but based on a stack trace found that certain API responses would sometimes come back processed as a string rather than an object while monitoring workflow logs.

Previous uncaught fatal exception:

$ terminus workflow:wait d9-aus.dev
PHP Fatal error: Uncaught Error: Attempt to assign property "id" on string in /Users/jms/pantheon/terminus-repo/src/Collections/TerminusCollection.php:124
Stack trace:
#0 /Users/jms/pantheon/terminus-repo/src/Commands/Workflow/WaitCommand.php(69): Pantheon\Terminus\Collections\TerminusCollection->fetch(Array)
#1 /Users/jms/pantheon/terminus-repo/src/Commands/Workflow/WaitCommand.php(44): Pantheon\Terminus\Commands\Workflow\WaitCommand->waitForWorkflow(1693326839, Object(Pantheon\Terminus\Models\Site), 'dev', 'Sync code on de...', 180)
#2 [internal function]: Pantheon\Terminus\Commands\Workflow\WaitCommand->workflowWait('d9-aus.dev', '', Array)
#3 /Users/jms/pantheon/terminus-repo/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array(Array, Array)
#4 /Users/jms/pantheon/terminus-repo/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#5 /Users/jms/pantheon/terminus-repo/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#6 /Users/jms/pantheon/terminus-repo/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(391): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#7 /Users/jms/pantheon/terminus-repo/vendor/symfony/console/Command/Command.php(298): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#8 /Users/jms/pantheon/terminus-repo/vendor/symfony/console/Application.php(1058): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /Users/jms/pantheon/terminus-repo/vendor/symfony/console/Application.php(301): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /Users/jms/pantheon/terminus-repo/vendor/symfony/console/Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /Users/jms/pantheon/terminus-repo/vendor/consolidation/robo/src/Runner.php(282): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /Users/jms/pantheon/terminus-repo/src/Terminus.php(486): Robo\Runner->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput), Object(Symfony\Component\Console\Application), Array)
#13 /Users/jms/pantheon/terminus-repo/bin/terminus(80): Pantheon\Terminus\Terminus->run()
#14 {main}
thrown in /Users/jms/pantheon/terminus-repo/src/Collections/TerminusCollection.php on line 124

Since I wasn't able to reproduce this, I added a handler for it to give more details of the error and a clearer failure.

$ terminus workflow:wait d9-aus.dev
 [error]  Fetch failed from /Users/jms/pantheon/terminus-repo/src/Commands/Workflow/WaitCommand.php:69, model_data expected as object but returned as string. String value: Hello World

@greg-1-anderson
Copy link
Member

I guess the overall cause here is inconsistent return values coming from guzzle?

@jms-pantheon
Copy link
Collaborator Author

@greg-1-anderson

I guess the overall cause here is inconsistent return values coming from guzzle?

I'm not sure on this, my suspicion is that the API is returning something like a plaintext 503 instead of JSON, but I can't reproduce so I can't prove that. I think once we start displaying the actual returned values as an error we'll be closer to knowing what's going on in this case.

@namespacebrian
Copy link
Contributor

I'd characterize it as.. "terminus currently assumes it will receive valid responses and unceremoniously crashes if its expectations aren't met. this PR checks the assumptions and deliberately return its own error when unexpected values are received, rather than letting the PHP runtime deal with it"

@jms-pantheon
Copy link
Collaborator Author

Alright here it is using print_r with truncation at 250 chars and a simulated array as an example.

[error]  Fetch failed /Collections/TerminusCollection.php:84 model_data expected as object but returned as array. Unexpected value: Array
(
    [string1] => Lorem ipsum dolor sit amet, consectetur adipiscing elit.
    [int1] => 42
    [float1] => 3.14159
    [boolean1] => 1
    [null1] =>
    [array1] => Array
        (
            [0] => apple
            [1] => banana
         ...

@jms-pantheon
Copy link
Collaborator Author

@greg-1-anderson when you have a chance to review, thank you

Copy link
Member

@greg-1-anderson greg-1-anderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's polish this a bit, doesn't look ready to go to customers yet.

namespacebrian and others added 2 commits September 20, 2023 13:02
* Adjust handling of TerminusCollection items lacking useful data

* Add comment clarifying handling of null response object
@jms-pantheon
Copy link
Collaborator Author

Alright cleaned up a bit further. Made variable names more clear, double checked the error logging.

@namespacebrian namespacebrian merged commit 55cc06e into 3.x Sep 27, 2023
7 checks passed
@namespacebrian namespacebrian deleted the missing-object branch September 27, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants