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

Webhook does not get fired anymore (if it failed before multiple times) #3146

Open
ndeet opened this issue Nov 22, 2021 · 10 comments
Open

Webhook does not get fired anymore (if it failed before multiple times) #3146

ndeet opened this issue Nov 22, 2021 · 10 comments
Labels
API Questions and issues related to API

Comments

@ndeet
Copy link
Contributor

ndeet commented Nov 22, 2021

BTCPay Server Version: v1.3.5

On a store I have registered 2 webhooks for different urls, one on beeceptor.com and one on a WooCommerce (btcpaywp.loca.lt) store. So each event fires on both domains.

Problem:

  • on the WooCommerce webhook when I return http status 500 (internal server error) for some time all retries fail which is fine
  • webhook endpoint returns 200 again
  • created a new invoice
  • no events fired to the previously failing webhook url (or they do not show up in the UI?)

Expected behaviour:

  • webhook should at least try to fire once for every new invoice?

Here is are the failed deliveries for btcpaywp... on invoice 1:
image

I created a new invoice, but only the other beecepter webhook deliveries fire:
image

After some time and or because I was checking the Webhooks details page about 16:40 the webhooks showed up as if they had been fired at 16:30 which can't be the case see second screenshot above:
image

Did not fully debug if the deliveres did fire at 16:30 but did not show up in the invoice details UI or if they fired after I visited the webhook details page. But then the timestamps and the order of deliveries occuring are wrong. I will do another testrun later and log the events to be sure what is happening exactly. But something does not work out here.

@pavlenex pavlenex added the API Questions and issues related to API label Jan 9, 2022
@NicolasDorier
Copy link
Member

NicolasDorier commented Jan 10, 2022

@ndeet there is a per website queue. BTCPay doesn't attempt delivery of events if previous one aren't delivered. This is to preserve event's order. Once the failing webhooks getting finally delivered OR that BTCPay store trying to redeliver it, you should then receive the next events.

I think the timestamp show not the delibery timestamp but the event timestamp. Will check.

This is actually the delivery's timestamp.

@NicolasDorier
Copy link
Member

Ok so what is happening is that the timestamp represent the timestamp when the delivery got created, not when it got sent.

In your case, the delivery was queued to preserve order of events. So that's why. Maybe we should add another field to delivery: "SentTimstamp"

@ndeet
Copy link
Contributor Author

ndeet commented Jan 10, 2022

@NicolasDorier thanks for investigating and explaining 💚 . Makes sense why the "old" timestamp shows up and agree that a second delivery/sent timestamp should be visible in the UI when the actual delivery happened.

@NicolasDorier
Copy link
Member

@pavlenex need to put that for after 1.4.0

@pavlenex pavlenex added this to the 1.5.0 milestone Feb 3, 2022
@pavlenex
Copy link
Contributor

@ndeet @NicolasDorier are we going to try to get this one for 1.5.0 or reschdule for the next one?

@ndeet
Copy link
Contributor Author

ndeet commented Apr 20, 2022

I forgot about that issue but needed to change the logic of the WooCommerce plugin as the way the queue works atm was causing trouble when the webhook endpoint returns a 404 because an invoice ID is not attached to an order anymore (can happen if the user goes back with browser and changes the payment method) or order was cancelled/deleted for any other reason. The issue is quite problematic in this case because of the delay the new invoice settlement does not get sent to the site anymore for some time. Longer explanation here https://chat.btcpayserver.org/btcpayserver/pl/bkfof1cf87fwzpcixf6krgxqhe

tldr; the queue should be by invoice id and not per webhook endpoint as 404 is a legit response and should not delay processing of all other store invoices, it will block also new invoice status updates from other customers atm.

@pavlenex pavlenex modified the milestones: 1.5.0, 1.6.0 Apr 27, 2022
@pavlenex pavlenex modified the milestones: 1.6.0, 1.7.0 Jul 14, 2022
@Kukks
Copy link
Member

Kukks commented Aug 26, 2022

I forgot about that issue but needed to change the logic of the WooCommerce plugin as the way the queue works atm was causing trouble when the webhook endpoint returns a 404 because an invoice ID is not attached to an order anymore (can happen if the user goes back with browser and changes the payment method) or order was cancelled/deleted for any other reason. The issue is quite problematic in this case because of the delay the new invoice settlement does not get sent to the site anymore for some time. Longer explanation here chat.btcpayserver.org/btcpayserver/pl/bkfof1cf87fwzpcixf6krgxqhe

tldr; the queue should be by invoice id and not per webhook endpoint as 404 is a legit response and should not delay processing of all other store invoices, it will block also new invoice status updates from other customers atm.

I think we should define our own handling spec for webhooks, and stating that the webhooks MUST always return a successful status code if it wishes to be acknowledged and not retried by btcpay.

Does that make sense @ndeet?

@ndeet
Copy link
Contributor Author

ndeet commented Aug 27, 2022

@Kukks agree that we should have some handling spec but not so sure about always expecting status code 200 even if something went wrong. E.g. when 404 not found happens then it should not retry, but if 500 internal server error happens it should as it may is a temporary problem. But there is also a point in your argument that even if the invoice ID is not found technically the webook was a success. Difficult decision 😬

@pavlenex pavlenex removed this from the 1.7.0 milestone Dec 19, 2022
@pavlenex
Copy link
Contributor

pavlenex commented Mar 3, 2023

Ping @ndeet and @Kukks what do we do with this one, it's been put on rescheduling spree since 1.5.0, is there a need to tackle this or not, if yes, then who can tackle it, if no, let's close or convert to a discussion.

@ndeet
Copy link
Contributor Author

ndeet commented Mar 8, 2023

Imo it would be good to have it sorted but it seems to be a major overhaul of the current way webhooks work and not that easy.

While this is likely not happening that often in the wild otherwise we would have many more complaints. But if there are merchants relying 100% on webhooks and have high volume checkout - when they have a temporary error for a few mins they will not get any payment confirmations until the next scheduled webhook run which could mean that quite some invoices stay unconfirmed in the meantime even they are paid and they have no clue why.

For WooCommerce plugin where it was a problem I worked around it by returning 200 status code even if the invoice was not found. (which is not that pretty but works for now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Questions and issues related to API
Projects
None yet
Development

No branches or pull requests

4 participants