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

update afragen/translations-updater with error caching/logging #199

Open
wants to merge 7 commits into
base: translations
Choose a base branch
from

Conversation

afragen
Copy link
Contributor

@afragen afragen commented Nov 16, 2024

Added API error caching for 60 min as GitHub API rate limits unauthenticated requests for 60 requests/hour. Leaves an entry in the debug.log

@afragen
Copy link
Contributor Author

afragen commented Nov 16, 2024

Fixes #200

@afragen afragen marked this pull request as ready for review November 16, 2024 22:26
@afragen
Copy link
Contributor Author

afragen commented Nov 16, 2024

Hopefully fixes #188

@ipajen
Copy link
Collaborator

ipajen commented Nov 17, 2024

@afragen

the #188 issue is solved in the fix.

But i am getting follow PHP warning when the 403 kicks in on language-pack.json

``Undefined property: stdClass::$name `

`wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/API.php:141
Fragen\Translations_Updater\Language_Pack_API->api()
wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Language_Pack_API.php:87
Fragen\Translations_Updater\Language_Pack_API->get_language_pack_json()
wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Language_Pack_API.php:56
Fragen\Translations_Updater\Language_Pack_API->get_language_pack()
wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Language_Pack.php:62
Fragen\Translations_Updater\Language_Pack->run()
wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Base.php:52
Fragen\Translations_Updater\Init->get_remote_repo_data()
wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Init.php:78
Fragen\Translations_Updater\Init->run()
wp-content/plugins/aspireupdate/aspire-update.php:76
aspireupdate_init_translations()
wp-includes/class-wp-hook.php:324
do_action('init')
wp-settings.php:704`

@afragen
Copy link
Contributor Author

afragen commented Nov 17, 2024

Well maybe it not quite the fix.

@afragen
Copy link
Contributor Author

afragen commented Nov 17, 2024

I'm thinking it might be better to return a WP_Error for a validation failure.

@ipajen
Copy link
Collaborator

ipajen commented Nov 17, 2024

Getting two PHP warnings now when 403 comes.

Undefined property: stdClass::$name | +wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/API.php:141

Undefined property: WP_Error::$slug | +wp-content/plugins/aspireupdate/vendor/afragen/translations-updater/src/Translations_Updater/Language_Pack.php:63

@ipajen
Copy link
Collaborator

ipajen commented Nov 17, 2024

But i see also in the debug logs

[17-Nov-2024 17:41:43 UTC] Translations Updater Error: (aspireupdate:main) - API rate limit exceeded for XXXIP. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)

@afragen
Copy link
Contributor Author

afragen commented Nov 17, 2024

Is this with current change back to returning false?

afragen@7c4774e

@asirota asirota added bug Something isn't working enhancement New feature or request and removed enhancement New feature or request labels Nov 17, 2024
@asirota asirota added this to the 0.6 milestone Nov 17, 2024
@afragen
Copy link
Contributor Author

afragen commented Nov 17, 2024

Just finished an update to the error handling. It should now degrade gracefully and document API timeout errors in the error log.

@ipajen
Copy link
Collaborator

ipajen commented Nov 18, 2024

Tested on the https://github.com/afragen/aspireupdate/tree/error-cache-logging bransch and the PHP warnings are gone and it’s caching if the rate limit is hit so it will not pound the GitHub API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Update afragen/translations-updater [Bug]: Attempt to read property "slug" on false
3 participants