PHP Fatal error in rates.php on line 168

[extrapixel]

PHP Fatal error: Cannot use object of type WP_Error as array in (...)/wp-content/plugins/wp-currencies/includes/rates.php on line 168

I don't know when this happens... but it's popping up in my error log sometimes.

[unfulvio]

Current L168 in rates.php is a blank line in master branch https://github.com/nekojira/wp-currencies/blob/master/includes/rates.php#L168
Does the error happen on a specific version? Does it still happen on the latest version?

[unfulvio]

if it's the latest version from WordPress - there was a minor commit in between on Github - so it could be actually this line for the version on WordPress.org

https://github.com/nekojira/wp-currencies/blob/master/includes/rates.php#L174

I'm not sure if wp_remote_get() returns an instance of WP_Error if it can't fetch results.

how frequent does it happen in logs? at the same interval it fetches currencies? do the currencies get updated in your db? do the requests on the stats on openexchangerates.org seem to match the requests that WP Currencies should make?

[extrapixel]

I'm on WP 4.2.2 (latest stable) and wp-currencies 1.4.0. Just filtered for rates.php (see screenshot). Actually it happens on line 71 and 168. The cron job seems to run, entries in the table have current timestamps and the rates seem legit.

just checking openexchangerates stats.... OOOPS

[unfulvio]

Oh wow... are you on a free account and getting these all from a single installation of WP Currencies or from elsewhere? Have you disclosed the API key somewhere? I think you're generating a bit less than a request per second on a day and the most WP Currencies should make is 1 request per hour...

[extrapixel]

just investigating. no, nothing else than wp-currencies should use the API. Cron is set to run hourly (even double checked the crons in the options table... all correct).
Probably someone using my API key. Arghhh. Anyways... probably the PHP Error comes from the API refusing to send data?

[unfulvio]

It's very likely, probably I need to make it fail more gracefully in such cases. I never run out of quota so I'm not sure what Openexchangerates returns in case of quota failure to pass the error message in WordPress.

If you published the API key somewhere, it's possible that someone used it as well. I double checked on cron.php that the most frequent interval in seconds is 1 day. I don't think currently the update method in the Rates class is called elsewhere other than cron. Unless it's called specifically in some custom code or the cron job is forced to run more frequently (or the cron is misconfigured on the server?).

If it's caused by WP Currencies, you should see that error at every second at least, like thousands of lines of the same error in your logs.

You could try to get a new app ID (the key) and see if the problem persist.

[extrapixel]

I did not knowingly publish it. As long as I'm over quota we could try to implement a better fail. Suggestions, where to have a look?

[extrapixel]

(i don't think the huge amount of requests is caused by wp-currencies, either)

[extrapixel]

fun fact though.... requests still increasing?! So I'm not sure if they actually do restrict usage?

[unfulvio]

To quickly fix the warnings issue I have made a new commit just now. It checks if the requests fails and if it's WP_Error won't proceed.

About your specific problem with the exceeded quota limit I'm not sure. But you can try changing the App ID to begin with.

[extrapixel]

Well, i deactivate wp-currencies and the flood of requests stopped. Could be coincidence - but I guess not. Trying to debug it later today.

[unfulvio]

I think the error originated from WP Currencies. I'm going to fix this asap.

[extrapixel]

Can I help you in any way? Put log-statements to some crucial places? Do you want my API-key for testing purposes?

[unfulvio]

I am very sorry. This happened only in 1.4.0. I had set a hook to update the currencies every time a new interval was saved in options. But instead of using hook 'updated_option_ I used 'update_option' a small typo, but made a big difference because it was being fired every time you called WP Currencies. This is now fixed with the last commit and I'm going to update the WordPress svn as well.

[extrapixel]

Nothing to be sorry about. Shit happens. Thanks for fixing :)

[unfulvio]

Please let me know how it goes with the plugin. I need to come up with a way to write proper tests for it by passing a key every time or in a local configuration file, since I don't want to put an API key in the repo.

[extrapixel]

I uninstalled the old version via wp gui, uploaded and installed the new version, added the api key, changed to hourly upates.

Result: 3 entries in wp-cron:

[1435164814] => Array
        (
            [wp_currencies_update] => Array
                (
                    [40cd750bba9870f18aada2478b24840a] => Array
                        (
                            [schedule] => hourly
                            [args] => Array
                                (
                                )

                            [interval] => 3600
                        )

                )

        )

[1435765783] => Array
        (
            [wp_currencies_update] => Array
                (
                    [40cd750bba9870f18aada2478b24840a] => Array
                        (
                            [schedule] => weekly
                            [args] => Array
                                (
                                )

                            [interval] => 604800
                        )

                )

        )

    [1435766007] => Array
        (
            [wp_currencies_update] => Array
                (
                    [40cd750bba9870f18aada2478b24840a] => Array
                        (
                            [schedule] => weekly
                            [args] => Array
                                (
                                )

                            [interval] => 604800
                        )

                )

        )

No data yet in wp-currencies table. Maybe first update will take an hour if set to hourly updates? Also, you should probably check for old cron-jobs before creating new ones (and remove them on deactivation and deletion!)

[unfulvio]

Uhm. On deactivation wp_clear_scheduled_hook( 'wp_currencies_update' ) it should be fired:

https://github.com/nekojira/wp-currencies/blob/master/includes/install.php#L53

Now I'm also clearing it before rescheduling (in case of settings change):

https://github.com/nekojira/wp-currencies/blob/master/includes/settings.php#L218
(*edited moved the clear schedule two lines above, so it doesn't do anything if there is no api key)

A few lines below you can see what happens when one saves the settings, it should schedule or reschedule the event (actually since few lines above it was cleared, that conditional wouldn't even be needed, but I'm leaving it there just in case).

From what I understood wp_schedule_event should fire anyway the first time when it's set (and i'm using time() for the first argument, which means 'now'. The second argument is the next run, which is set by the user, the third argument is the hook, in this case is... go to update the rates):
https://codex.wordpress.org/Function_Reference/wp_schedule_event

[extrapixel]

mhhhh okey. Request flood has started again, too. Weird. I'll clear the crons manually now and see what happens.

[extrapixel]

Okey i inserted a debug message right before the requests for currency lists and rates.

[25-Jun-2015 19:48:26 UTC] remote get the currency rates
[25-Jun-2015 19:48:26 UTC] remote get currency list
[25-Jun-2015 19:48:26 UTC] remote get the currency rates
[25-Jun-2015 19:48:26 UTC] remote get currency list
[25-Jun-2015 19:48:27 UTC] remote get the currency rates
[25-Jun-2015 19:48:27 UTC] remote get currency list
[25-Jun-2015 19:48:27 UTC] remote get the currency rates
[25-Jun-2015 19:48:27 UTC] remote get currency list
[25-Jun-2015 19:48:27 UTC] remote get the currency rates
[25-Jun-2015 19:48:27 UTC] remote get currency list
[25-Jun-2015 19:48:28 UTC] remote get the currency rates
[25-Jun-2015 19:48:28 UTC] remote get currency list
[25-Jun-2015 19:48:28 UTC] remote get the currency rates
[25-Jun-2015 19:48:28 UTC] remote get currency list
[25-Jun-2015 19:48:29 UTC] remote get the currency rates
[25-Jun-2015 19:48:29 UTC] remote get currency list

So it definitely is wp-currencies making all these requests.

[unfulvio]

but even after the latest updates from yesterday? I changed the action hook - I don't understand why the two methods in rates.php are being called even when the site is not moving

update_rates is the callback for the wp cron job defined in cron.php - the Cron class schedules the action hook wp_currencies_update and it should only fire the first time it's set (or options updated) and then at the set interval frequency (hourly, weekly, etc).

what's your idea?

[unfulvio]

The only possibility that I'm still using the wrong action hook - I chose options_updated, which should be fired only when... options are updated - and the callback for it has a wrapper that checks if the $option passed is wp_currencies_settings, ie. it's WP Currencies options that are saved.

[unfulvio]

to be honest even the previous action hook looked like correct - 'update_option_wp_currency_settings', it should have fired only when wp_currency_settings is updated and saved to db... but I thought it was the wrong hook

[unfulvio]

ok I reverted to the previous hook after checking WP codebase... it was fine

instead, I'm trying now to wrap the cron action callback with a wrapper that checks if WordPress is doing a cron job:
https://github.com/nekojira/wp-currencies/blob/master/includes/cron.php#L42

although I think that wasn't necessary, this would stop running the update unless it's a cron job

caveats:

• if the problem is that the cron job is run every second or so, this won't still work
• if you are on alternate cron this might never be executed and the plugin won't work

[unfulvio]

we are continuing this discussion on #6 where I posted the latest updates

[unfulvio]

@extrapixel please try with the latest plugin version I think the issue is finally solved