donaldcameron

Forum Replies Created

Viewing 4 replies - 1 through 4 (of 4 total)
  • Forum: Plugins
    In reply to: [WooCommerce] order.created webhook firing twice
    Thread Starter donaldcameron

    (@donaldcameron)

    9 months ago

    Done: https://github.com/woocommerce/woocommerce/issues/60886.

    Forum: Plugins
    In reply to: [WooCommerce] order.created webhook firing twice
    Thread Starter donaldcameron

    (@donaldcameron)

    9 months ago

    Cheers for the response, Moses!

    And, yeah, I get all that. I did actually even quote the part of the docs that explains it.

    I suspect you have not actually seen my most recent response? I do explain what the story is, include relevant code etc.

    But to summarise again:

    It’s not the _emptiness of the body_ that is the issue. It’s the request doesn’t send all the headers _that I’ve configured it to_ when doing the ping. It’s not sending the auth. This makes it not a valid way to perform the test it’s trying to. I pointed out the bit of code and what’s missing.

    Regarding this:

    A practical way to handle both real and validation requests is to modify your webhook endpoint

    I amn’t using WP on the other end, and also *simply letting any old unauthorised request through to my app* is not a great suggestion, security-wise. The security on the end point says “it needs to be a post, and it needs to have valid auth. Fullstop”. The request doesn’t even get to my controller method; the routing & security systems go “nuh-uh” well before that. This is how these things tend to work in 2025, I think? It’s not for the controller to deal with this stuff on an ad hoc basis.

    I worked around it by updating the DB field when we first install the webhook. This is fine. If the webhook’s target _fails_ subsequently, we’ll get all the logging and alerts and stuff.

    Forum: Plugins
    In reply to: [WooCommerce] order.created webhook firing twice
    Thread Starter donaldcameron

    (@donaldcameron)

    9 months ago

    More info again.

    I have found the code that runs the ping, and the code that makes the delivery request, and compared them.

    https://woocommerce.github.io/code-reference/files/woocommerce-includes-class-wc-webhook.html#source-view.603

    	public function deliver_ping() {
    		$args = array(
    			'user-agent' => sprintf( 'WooCommerce/%s Hookshot (WordPress/%s)', Constants::get_constant( 'WC_VERSION' ), $GLOBALS['wp_version'] ),
    			'body'       => 'webhook_id=' . $this->get_id(),
    		);

    		$test          = wp_safe_remote_post( $this->get_delivery_url(), $args );

    ^^^ does not add the additional headers.

    Contrast with the correct approach in https://woocommerce.github.io/code-reference/files/woocommerce-includes-class-wc-webhook.html#source-view.343:

    public function deliver( $arg ) {
    	// SNIP
    	
    	// Setup request args.
    	$http_args = array(
    		// SNIP
    	);

    	$http_args = apply_filters( 'woocommerce_webhook_http_args', $http_args, $arg, $this->get_id() );

    	// Add custom headers.
    	// SNIP

    	// Webhook away!
    	$response = wp_safe_remote_request( $this->get_delivery_url(), $http_args );

    Note how deliver calls apply_filters there. deliver_ping should be doing the same.

    Can’t see how this isn’t just a bug?

    I’ve had a look through the code and the docs for the CLI and API and can see nowhere that exposes pending_delivery as a settable. There’s a (couple of ~) set_pending_delivery methods and calls thereto, but it’s all internal stuff.

    I’ll bodge this by hitting the DB directly.

    Forum: Plugins
    In reply to: [WooCommerce] order.created webhook firing twice
    Thread Starter donaldcameron

    (@donaldcameron)

    9 months ago

    More info.

    There’s a pending_delivery column in the wp_wc_webhooks table. This is set to 1 by default, when the wp wc webhook create is run. Fair enough I s’pose: it’s not been tested / pinged yet.

    If the “second request” gets a 200-OK, it’s set to 0, and the second requests stop. It does not seem to pay attention to the result of the actual webhook requests (which always respond with a 200-OK) to determine whether to change this pending status; it’s only paying attn to the second request.

    If I reset it to 1… the second requests start again.

    If I manually set it to 0, then the second request does not fire.

    So it seems it’s something to do with this from the UI docs:

    4/ Save webhook.

    Note: The first time your webhook is saved with the Activated status, it sends a ping to the Delivery URL.

    I bet the “second request” is this ping.

    It looks like there’s a slight glitch in the logic when it comes to WC deciding whether it needs to ping or not. I think it should only ever really be done once – when the webhook is first created – it should not continue to try pinging along with the actual webhook request, irrespective of the value of pending_delivery value. It should use the actual webhook request’s result! At the end of the day, that’s what matters after all. Potential bug #1.

    Also the ping should form the request properly… if the “actual” webhook request picks up the extra (necessary) headers, then the ping should too! Probable bug #2.

    Anyhoo… even if ppl on the WC end conclude that there are bugs here (I’m not saying there are, but it is looking that way), I don’t expect an immediate fix: we’re all busy.

    As our install of the WP/WC site is all scripted, I can just sling a 0 into that table row. I can’t find anything in the webhooks docs (which are very end-user / UI-centric, unless me and google just can’t find the treasure trove of dev-centric docs?) that indicate a way of disabling the ping, so poss an SQL UPDATE statement will need to be the order of the day :-S

    BTW, Frank: thanks for your quick response! I know I have not answered your Qs, but I thought this further info probably casts better light on the scene.

Viewing 4 replies - 1 through 4 (of 4 total)