Skip to content

Don't report a cancelled task as a refused refund - #44

Open
zlexdev wants to merge 2 commits into
funpayhub:devfrom
zlexdev:fix/refund-bare-except
Open

Don't report a cancelled task as a refused refund#44
zlexdev wants to merge 2 commits into
funpayhub:devfrom
zlexdev:fix/refund-bare-except

Conversation

@zlexdev

@zlexdev zlexdev commented Aug 30, 2026

Copy link
Copy Markdown

Refund.parse_result catches everything — funpaybotengine/methods/refund.py:42-49:

try:
    result = json.loads(response.raw_response)
except:
    raise RefundError(
        order_id=self.order_id,
        message=f"Unable to refund order {self.order_id}",
    )

CancelledError and KeyboardInterrupt derive from BaseException, so:

task = asyncio.create_task(bot.refund(order_id="..."))
task.cancel()          # -> RefundError("Unable to refund order ...")

The order may already be refunded. Refused and unknown arrive as the same
exception, and a caller retrying on RefundError retries a refund that went through.

The reason is dropped too — no from, nothing of the decode error — so an HTML login
page and a truncated body produce one identical sentence.

What changes

json.JSONDecodeError is caught specifically, its text goes into the message, and the
cause is chained. loggers.py gains methods_logger, following the one-per-subsystem
shape already there (session, router, dispatcher, runner); methods had none.

The raw body goes to DEBUG only, capped at 500 characters with the full length printed
beside it — when the answer is a login page it is a whole HTML document, and every failed
refund would put one in the log.

Tests

tests/test_refund.py, four tests. On the parent commit:

FAILED tests/test_refund.py::test_an_unparsable_answer_carries_why_and_chains_the_cause
FAILED tests/test_refund.py::test_cancellation_is_not_a_refused_refund
2 failed, 2 passed

ruff and mypy clean on the touched files. Pre-existing findings elsewhere are left
alone.

`Refund.parse_result` caught everything:

    except:
        raise RefundError(order_id=..., message=f'Unable to refund order {...}')

`CancelledError` and `KeyboardInterrupt` derive from `BaseException`, so a task
cancelled mid-refund surfaces as "unable to refund" for an order whose request
may already be in flight. The two states a caller must tell apart -- refused and
unknown -- become one.

The message drops the reason as well: no `from`, and nothing of the decode
error, so an HTML login page and a truncated body are the same sentence.

Now:

    except json.JSONDecodeError as exc:
        methods_logger.debug('refund %s: response is not JSON: %r', ...)
        raise RefundError(..., message=f'Unable to refund order {...}: {exc}') from exc

`methods_logger` is new and follows the existing one-per-subsystem shape in
`loggers.py`; `methods` had none. The raw body goes to DEBUG only -- it is what
says why parsing failed, and it is also a full HTML page when the answer was a
login form.

`tests/test_refund.py`: four tests, two of which fail on the parent commit --
the chained cause and the cancellation.
@zlexdev
zlexdev requested a review from qvvonk as a code owner August 30, 2026 12:44
A refused refund answered with a login page put a whole HTML document in the
log, once per failure. The head identifies it; the length is printed beside it
so nothing is hidden without saying so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant