Uncaught GuzzleHttp\Exception\ConnectException in remote post hander

Created on 8 January 2024, 12 months ago

Problem/Motivation

While using the remote post webform handler, we got a WSOD because of an uncaught ConnectionException:

GuzzleHttp\Exception\ConnectException: cURL error 28: Operation timed out after 30000 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api-dev.stellahealth.net/connectflatform/forms/clientsurvey in GuzzleHttp\Handler\CurlFactory::createRejection() (line 210 of /var/www/html/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php).

Steps to reproduce

Get unlucky while using remote post.

Proposed resolution

This appears to be a result of a change made in Guzzle 7. This core issue looks closely related: #3188920: Make Guzzle exception handling forward-compatible with Guzzle 7 β†’

The remote post handle catches a RequestException (https://git.drupalcode.org/project/webform/-/blame/6.2.x/src/Plugin/Webf...)

    catch (RequestException $request_exception) {
      $response = $request_exception->getResponse();

      // Encode HTML entities to prevent broken markup from breaking the page.
      $message = $request_exception->getMessage();
      $message = nl2br(htmlentities($message));

      $this->handleError($state, $message, $request_url, $request_method, $request_type, $request_options, $response);
      return;
    }

In Guzzle 7 though, ConnectException is a sibling and not an extension on RequestException.

I'd love to just change RequestException to TransferException (the parent of both RequestException and ConnecException) but ConnectException does not have a getResponse method.

So we might need to add a new catch block.

Remaining tasks

User interface changes

N/A

API changes

N/A

Data model changes

N/A

πŸ› Bug report
Status

Active

Version

6.2

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @danflanagan8
  • Status changed to Needs review 12 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    511 pass, 38 fail
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Here's a basic fail test.

  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    I'm not sure why there were 38 failures (?!) but here's the one I was trying to introduce:

    Webform.Drupal\Tests\webform\Functional\Handler\WebformHandlerRemotePostTest
    βœ—	
    Drupal\Tests\webform\Functional\Handler\WebformHandlerRemotePostTest
    exception: [Other] Line 0 of sites/default/files/simpletest/phpunit-48.xml:
    PHPUnit Test failed to complete; Error: PHPUnit 9.6.15 by Sebastian Bergmann and contributors.
    
    Testing Drupal\Tests\webform\Functional\Handler\WebformHandlerRemotePostTest
    E                                                                   1 / 1 (100%)
    
    Time: 00:09.847, Memory: 4.00 MB
    
    There was 1 error:
    
    1) Drupal\Tests\webform\Functional\Handler\WebformHandlerRemotePostTest::testRemotePostHandler
    Behat\Mink\Exception\ExpectationException: The string "Unable to process this submission. Please contact the site administrator." was not found anywhere in the HTML response of the current page.
    
    /var/www/html/vendor/behat/mink/src/WebAssert.php:794
    /var/www/html/vendor/behat/mink/src/WebAssert.php:324
    /var/www/html/core/tests/Drupal/Tests/WebAssert.php:540
    /var/www/html/modules/contrib/webform/tests/src/Functional/Handler/WebformHandlerRemotePostTest.php:170
    /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
    
  • Status changed to Needs review 12 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    512 pass, 36 fail
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Here's a fix.

    I also added assertions for the exception messages.

  • Status changed to Needs review 12 months ago
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    I have no idea what's causing those other 36 test fails. I think that's unrelated, so I'm going to flip this back to NR.

  • I think I've seen those failures on other webform issues. HEAD could be failing.

  • Status changed to Postponed: needs info 9 months ago
  • πŸ‡ΊπŸ‡ΈUnited States jrockowitz Brooklyn, NY

    This MR could be creating config validation issues because the remote post status code could now be a string.

    We should try to see if the No Response status code addresses this concern/issue.

    @see πŸ› Improve error handling for unreachable endpoints while using Remote Post handlers Fixed

  • Status changed to Needs review 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Hi @jrockowitz!

    > This MR could be creating config validation issues because the remote post status code could now be a string.

    I don't think that's a problem here.. There's no schema related to those changes to webform.webform.test_handler_remote_post.yml. It's part of multiline string in the yaml. It's kind of funky, but it sseemed like the path of least resistance to have some decent test coverage for these Guzzle exceptions.

    > We should try to see if the No Response status code addresses this concern/issue.

    Looking at the commit for that issue, it doesn't look like that will totally solve the problem with the guzzle exception. There are no changes to the code that runs prior to this uncaught exception. The issue here is fundamentally about a change made in Guzzle 7 to the structure of exceptions. I think it needs its own solution.

  • πŸ‡¦πŸ‡ΊAustralia tinny

    Confirming this patch works for me.

Production build 0.71.5 2024