composer-wp-pro-plugins icon indicating copy to clipboard operation
composer-wp-pro-plugins copied to clipboard

Improve error handling and logic

Open mcaskill opened this issue 1 year ago • 4 comments

Checklist

  • [x] I've read the Contributing page.
  • [x] Issues: #54, #55, #56, #57
  • [x] My code is tested.
  • [x] My code follows the WordPress code style.
  • [x] My code has proper inline documentation.

Draft 2 — 2023-03-15

Description

Follow-up to #52 and alternative to the initial draft (see below).

Primary focus is improving error handling by validating the response from cURL requests, validating the decoded/unserialized response bodies, and simplifying the logic between plugins.

How has this been tested?

I'm testing this on client projects that use ACF Pro, ACF Extended Pro, PublishPress Pro, Gravity Forms, Polylang Pro, and WPML.

Screenshots

Screen capture of the originally proposed error example "cURL error (22): The requested URL returned error: 404"

Screen capture of the latest proposed error example "JSON Error: Expected a data structure"

Types of changes

  • Plugins:
    • ACF Pro, Gravity Forms, WPML: Change URL formatting to use http_build_query() to improve readability and reduce risk of mistakes.
    • ACF Extended Pro, Gravity Forms, Polylang Pro: Change HTTP request method to use GET instead of POST to better match the verb of the request and for consistency between the other downloader. Both Gravity Forms' API and Easy Digital Downloads' API support GET requests.
    • Ninja Forms, PublishPress Pro, WPML: Throw InvalidArgumentException instead of UnexpectedValueException when dealing with an unsupported package.
    • EDD Plugins, Gravity Forms: Throw UnexpectedValueException if the decoded/unserialized API response is not an array.
    • EDD Plugins: Replaced getDownloadUrl() with a new protected method getDownloadUrlFromApi() that only handles the request for the download URL response object.
  • AbstractEddPlugin:
    • Replaced extractDownloadUrl() with getDownloadUrl() that retrieves the response to parse from the plugin's getDownloadUrlFromApi().
    • Wrapped requests with Http in a try/catch to intercept cURL exceptions and wrap them in a plugin-aware exception.
  • Http:
    • Added protected method request() to aggregate the execution of the request, handling of the response, and closing of the cURL handler.
    • Added cURL option CURLOPT_FAILONERROR to fail if the response is >= 400.
    • Throw RuntimeException if the request failed.
    • Compile URL in GET request earlier to organize cURL options similarly to POST request.
    • Use booleans intead of integers for cURL options.

Draft 1 — 2023-03-07

Description

Follow-up to #52.

Fixed the PSR-4 file name for Wpml class, for the sake of testing.

Improve checks on HTTP response. Check if status code is OK (200) and if the body is an array, throw exceptions if not with an excerpt of the response body for aid with debugging.

Expanding upon #47, improved Gravity Forms to check if download version matches the package's version with support for either available download: the "main version" (download_url) or the "latest version" (download_url_latest). For example:

  • version: 2.7.2 (PARADIGM.MAJOR.MINOR)
  • version_latest: 2.7.2.1 (PARADIGM.MAJOR.MINOR.PATCH)

As of 2023-03-08, this code sort of works but is not completely tested/vetted, nor is it the best design, and some of it should be split into their own pull requests.

If we drop support for Composer 1, a lot of this proposed code can be simplified to take advantage of Composer 2's utilities.

I'm open to comments, suggestions, and contributions.

How has this been tested?

I'm testing this on client projects that use ACF Pro, ~~ACF Extended Pro~~, ~~PublishPress Pro~~, Gravity Forms, Polylang Pro, and WPML.

Types of changes

  • Change URL formatting (for ACF, Gravity Forms, WPML) to use http_build_query() to improve readability and reduce risk of mistakes.
  • Change HTTP request method (for ACF Extended, Gravity Forms, Polylang) to use GET instead of POST to better match the verb of the request and for consistency between the other downloader. Both Gravity Forms' API and Easy Digital Downloads' API support GET requests.
  • Refactored Http class to store last response body and headers, allow parsing of body through callback, and finding of status code and message (based on Composer 2's Response, RemoteFileSystem, and HttpDownloader).
  • Use Composer's Semver to check Gravity Forms versions and throw an exception if we download the incorrect version.

mcaskill avatar Mar 08 '23 23:03 mcaskill

Looks good @mcaskill are you still working on this?

junaidbhura avatar Mar 12 '23 21:03 junaidbhura

Thanks. Unfortunately, at this time, I won't be able to continue/finish work on this feature. If you or someone else wants to take over, I can add them to the branch.

~~Similarly, could you fix the WPML class (WPML.phpWpml.php) to resolve the open issue?~~

mcaskill avatar Mar 13 '23 13:03 mcaskill

I've updated the pull request title and description to reflect the latest state of proposed changes.

The initially proposed changes for Gravity Forms will be resubmitted as its own pull request once we've figured out this request.

mcaskill avatar Mar 16 '23 00:03 mcaskill

I found some time to do some further testing and improvements.

I think it should be functional enough.

If people can test, that would be appreciated.

mcaskill avatar Jun 09 '23 20:06 mcaskill