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

Make it easier to determine if we are 'inside' a queue processor #1024

Open
remcotolsma opened this issue Jan 18, 2024 · 5 comments
Open

Make it easier to determine if we are 'inside' a queue processor #1024

remcotolsma opened this issue Jan 18, 2024 · 5 comments
Labels
priority: normal The issue/PR is normal priority—not many people are affected or there’s a workaround, etc. type: enhancement The issue is a request for an enhancement.

Comments

@remcotolsma
Copy link
Contributor

remcotolsma commented Jan 18, 2024

It could be useful to other developers if we make it easier to detect when code is executing in the context of the Action Scheduler queue. There are various patterns/strategies to determine this already, but it's not quite as straightforward as it might be. We should at least consider the proposal in this comment. Original description follows.


We know that WordPress sometimes increases the timeout of HTTP requests when initiated via the WordPress cron.

$options = array(
	'timeout' => wp_doing_cron() ? 30 : 3,
);

See for example:

We'd like to do something similar for tasks initiated through this library. However, it seems that wp_doing_cron() does not always return true when running "Action Scheduler" tasks. In the codebase of this library there is also no define( 'DOING_CRON', true ); as in WordPress:

/**
 * Tell WordPress the cron task is running.
 *
 * @var bool
 */
define( 'DOING_CRON', true );

https://github.com/WordPress/WordPress/blob/f4b5da285990e948e7b40ba53be5037aed2ae07b/wp-cron.php#L37-L42

Now we might be able to catch this with a piece of code as follows:

\add_action( 'action_scheduler_run_queue', function () {
	if ( \defined( 'DOING_CRON' ) ) {
		return;
	}

	define( 'DOING_CRON', true );
} );

However, we wonder whether this is wise and/or correct, so we are curious about the opinion of the developers of this library. Should DOING_CRON be set to true when running "Action Scheduler" tasks? Or is it useful/better to introduce an as_doing_cron() function or something like that? Or are we looking at this completely wrong?

CC @rvdsteege

@barryhughes
Copy link
Member

Hi!

However, it seems that wp_doing_cron() does not always return true when running "Action Scheduler" tasks.

That's right. By default, there are up to three different contexts through which Action Scheduler may be triggered:

  • WP Cron
  • Async HTTP requests
  • WP CLI

In most cases, it's a mix of the first two: it doesn't always run via WP Cron (and so DOING_CRON will not always be set).

However, we wonder whether this is wise and/or correct, so we are curious about the opinion of the developers of this library.

If you are finding you need something like this, then possibly? Can I just highlight though that we already have some related functionality within the library:

https://github.com/woocommerce/action-scheduler/blob/3.7.1/classes/ActionScheduler_Compatibility.php#L87-L104

This is generally called immediately prior to processing waiting actions. However, there isn't really any way to ensure this takes effect: restrictions in a given hosting environment take precedence, in many cases.

@remcotolsma
Copy link
Contributor Author

remcotolsma commented Jan 19, 2024

Thanks @barryhughes, good to know about these 3 contexts. I think the PHP timeouts you were referring to are different from the WordPress HTTP API timeouts. WordPress uses a standard timeout of 5 seconds for HTTP requests: https://developer.wordpress.org/reference/classes/wp_http/request/#parameters. For HTTP requests that are executed via background tasks, this may be slightly higher. However, there is currently no easy way to check whether a script is running in one of the 3 contexts mentioned? For the WordPress cron wp_doing_cron() for WP-CLI defined( 'WP_CLI' ) && WP_CLI and for the Action Scheduler "Async HTTP requests"?

@barryhughes
Copy link
Member

I'm sorry, I misunderstood.

However, there is currently no easy way to check whether a script is running in one of the 3 contexts mentioned?

I'm not sure how useful this would be in your case, but the action_scheduler_begin_execute hook is triggered immediately before each scheduled action is fetched and executed, and supplies two parameters to its callbacks:

  • $action_id
  • $context

The second of those provides the information you are looking for (and even without it, you could still use the action itself as a signal that you are 'inside' the scheduled action queue).

Might that work? If not, we could explore alternatives. For instance, it would seem reasonable to also share the context via the action_scheduler_before_process_queue action hook, and there are probably various other possibilities.

@lsinger lsinger added type: enhancement The issue is a request for an enhancement. type: support request Issues submitted that are not bugs or enhancements but support requests (both dev and non-dev). labels Jan 22, 2024
@remcotolsma
Copy link
Contributor Author

@barryhughes
Copy link
Member

It would be nice if this library introduced it's own DOING_ACTION_SCHEDULER or something similar.

Re-opening so we don't lose sight of this idea (a little unsure if defining a constant is the best approach for us, but the general idea makes sense).

@barryhughes barryhughes reopened this Jan 23, 2024
@barryhughes barryhughes added priority: normal The issue/PR is normal priority—not many people are affected or there’s a workaround, etc. and removed type: support request Issues submitted that are not bugs or enhancements but support requests (both dev and non-dev). labels Jan 23, 2024
@barryhughes barryhughes changed the title Increase WordPress HTTP API timeout in case of "Action Scheduler" background tasks via e.g. wp_doing_cron() / DOING_CRON Make it easier to determine if we are 'inside' a queue processor Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: normal The issue/PR is normal priority—not many people are affected or there’s a workaround, etc. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants