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

Improve exception handling #12

Merged
merged 1 commit into from
Aug 14, 2018
Merged

Improve exception handling #12

merged 1 commit into from
Aug 14, 2018

Conversation

swissspidy
Copy link
Contributor

This PR:

  • accounts for the fact that the API returns a JSON object with an error property when a plugin is not available.
  • Uses Composer's RemoteFileSystem class to handle downloads. It does retries, exception handling, etc.
  • Displays a warning when getting a TransportException (which likely occurs when w.org is down).

To do:

  • Allow passing RemoteFileSystem via constructor so we can use RemoteFilesystemMock in tests and verify the correctness of this functionality.
  • Maybe only print this warning once, ideally after all updates/installs went through.

See #5.

@swissspidy swissspidy requested a review from ocean90 February 6, 2018 12:44
@swissspidy
Copy link
Contributor Author

@ocean90 When you've got a minute next week :-)

Copy link
Member

@ocean90 ocean90 left a comment

Choose a reason for hiding this comment

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

Only noticed that the error message can be displayed twice if the API was down for both requests.

Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
Could not reach WordPress.org to verify plugin availability status.
Could not reach WordPress.org to verify plugin availability status.
  - Updating wpackagist-plugin/spotify-embed (0.1 => 0.2): Downloading (100%)
    Changelog: https://wordpress.org/plugins/spotify-embed/#developers
Writing lock file
Generating autoload files

@swissspidy
Copy link
Contributor Author

Only noticed that the error message can be displayed twice if the API was down for both requests.

Yeah that's something I want to change next (see todo section in original PR description).

@swissspidy swissspidy merged commit aa648e6 into master Aug 14, 2018
@swissspidy swissspidy deleted the 5-exceptions branch August 14, 2018 06:40
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.

2 participants