fix: raise failed request to not run mark_webhook_sent

[dni]

error is not raised, even if failed webhook would be marked as sent every time

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 58.55%. Comparing base (5b43989) to head (cf9bd03).

Files	Patch %	Lines
lnbits/core/tasks.py	0.00%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #2289      +/-   ##
==========================================
- Coverage   58.63%   58.55%   -0.09%     
==========================================
  Files          61       61              
  Lines        9192     9205      +13     
==========================================
  Hits         5390     5390              
- Misses       3802     3815      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[motorina0]

In dispatch_webhook the status code is set to -1 in case of failure. We should be consistent in handling the error.

        try:
            r = await client.post(payment.webhook, json=data, timeout=40)
            await mark_webhook_sent(payment, r.status_code)
        except (httpx.ConnectError, httpx.RequestError):
            await mark_webhook_sent(payment, -1)

There are 3 places where this pattern ☝️ is repeated.
A common function should be extracted: call_webhook(payment, data, timeout)

[dni]

i was actually misinterpreting the code, we don't want to raise an HTTPStatusException here, because we actually want to set the status code to 404 or 500 if it fails to see if the response was not ok. we just want to set status -1 when we cannot even send the request.

i also saw that mark_webhook_sent is actually saved twice, for payment.webhook and for balance_notify which i think is kind of weird.

[motorina0]

Is httpx.ConnectError not ever thrown?
Can we just catch Error and log the actual error?
Is there a case when the error should bubble up (and stop the loop)?

[dni]

ConnectError is capture by RequestError look at the exception tree here. it was redundant. https://www.python-httpx.org/exceptions/

[dni]

statuserror is triggerbe by raise_for_status() and is raised if the response has a bad status code

[motorina0]

Which is the difference between the two catch clauses (httpx.HTTPStatusError vs httpx.RequestError), why are they handled differently?

[dni]

request error is the request never reached the server, httperror is it hits the server but returns a bad statuscode

[motorina0]

why are they handled differently though?

[dni]

because we want to save the bad statuscode inside the payment details, it was like that before. so we know we did send it, but the receiver couldnt handle it.