Skip to content

[FIX] pos_hobex: no crash on hobex timeout, poll transaction state instead of starting a new payment (18.0) - #2

Open
geraldaistleitner wants to merge 1 commit into
Callino:18.0from
geraldaistleitner:18.0-fix-pos-hobex-timeout-handling
Open

geraldaistleitner wants to merge 1 commit into
Callino:18.0from
geraldaistleitner:18.0-fix-pos-hobex-timeout-handling

Conversation

@geraldaistleitner

Copy link
Copy Markdown

Forward-port of #1 (17.0) to 18.0, adapted to the RPC-proxy architecture of this branch.

Problem

hobex_start_sync_transaction still crashes in its own except handler on 18.0 when hobex does not answer the payment request within the 80 s read timeout:

  File ".../pos_hobex/models/pos_payment_method.py", line 189, in hobex_start_sync_transaction
    }, response or None
UnboundLocalError: local variable 'response' referenced before assignment

response is only assigned inside the try, so whenever requests.post itself raises the handler hits an unbound name. On 18.0 the exception propagates out of proxy_hobex_payment_request, pos.data.silentCall swallows it and resolves with false, and _hobex_handle_payment_request_done then reads false.responseCode and shows "undefined: undefined". Odoo core puts the line into retry, so the cashier can only delete the card line (and rebook as cash) or press Send again, which starts a second hobex transaction while the first one is frequently still completing at the terminal. We have had to repair several 17.0 production orders where the card charge went through but the order was finalized with a cash payment.

While porting I found three more client-side problems on this branch that make the recovery path unusable:

  • _hobex_handle_status_update_response stores hobex_responseCode = "0" before checking the state. After an INPROGRESS answer the next Send therefore takes the "transaction was already successful" branch and marks an unfinished payment as paid.
  • The status-poll branch in send_payment_request calls _hobex_update_payment_status(order, uuid) without chaining its promise, so the outer promise never resolves and the line hangs. The branch was unreachable anyway because "hobex_responseCode" in line is always true (models.js initialises the field to false).
  • _hobex_handle_status_connection_failure returns Promise.reject and never resolves; a null reversal result (hobex timeout) crashes _hobex_handle_reversal_response.

Changes

Server

  • hobex_start_sync_transaction: transaction lookup moved out of the try. On any exception the transaction now stays pending with the error in message and the method returns responseCode -1. It no longer guesses failed: after a timeout the outcome of a card payment is unknown and the customer may still complete it at the terminal. Timeouts and connection errors get a readable message.
  • proxy_hobex_status_request: error results carry a code (not_found / no_answer) so the POS can tell "hobex never got this transaction, start a new one" from "hobex unreachable, ask again". An exception from update_hobex_state is caught and reported as no_answer instead of leaking through silentCall as false.
  • hobex_reversal_transaction: return (None, None) on exceptions (as on 19.0) instead of None, which made proxy_hobex_reversal_request raise on unpacking.
  • _update_transaction_with_hobex_result
    • the signature-receipt download (cvm == 1) is wrapped in its own try/except, so a receipt hiccup can no longer turn a successful payment into a -1 error
    • the state mapping evaluates state first and falls back to responseText, with a safe default of ok plus a warning for unknown values. The 18.0 code left state unassigned (NameError) for any value other than OK/VOID/INPROGRESS. I evaluate state first because the POS client keys on that field; please double check against a real v2 status response.
  • update_hobex_state: res.get('state').

POS client (payment_hobex.js)

  • A -1 answer is no longer stored as the line's hobex result. The client immediately asks proxy_hobex_status_request for the real state (the server waits for the terminal itself) and resolves from that: success → done with all card details, abort/failure → retry, transaction id reset so a new attempt is possible.
  • A pending line without a hobex result polls the state on the next Send instead of starting a new transaction; the outer promise is resolved from the poll result.
  • INPROGRESS no longer stores a response code; the cashier gets a German message to finish at the terminal and press Send again.
  • not_found resets the transaction id; no_answer keeps it.
  • All handlers cope with false/null results from silentCall and always resolve, so a line can no longer hang.
  • The line stays removable in every error case (retry), so a hobex outage never blocks finishing an order.

Version bumped to 18.0.2.1.1.

Testing

Syntax checked (py_compile, node --check); not runtime-tested on 18.0, we are preparing our upgrade from 17.0. The scenario is easy to reproduce with a real terminal by not presenting a card for ~90 s: the line should end done once the customer taps, and 8004 should be reported when the transaction is aborted at the terminal.

…tate instead of restarting

Forward-port of the 17.0 fix, adapted to the RPC proxies of this branch.

- hobex_start_sync_transaction: `response` was referenced in the except handler
  before assignment when requests.post itself raised (e.g. read timeout after 80s)
  -> UnboundLocalError. The transaction now stays pending (the outcome is unknown,
  the customer may still finish at the terminal) and responseCode -1 is returned.
- proxy_hobex_status_request: error results carry a code (not_found / no_answer),
  exceptions from update_hobex_state are reported as no_answer.
- hobex_reversal_transaction: return (None, None) on exceptions.
- _update_transaction_with_hobex_result: receipt download (cvm=1) can no longer
  turn a successful payment into an error; state mapping evaluates 'state' first
  and no longer raises NameError for unknown values.
- POS: a -1 answer no longer counts as a hobex result - the client asks for the
  state of the existing transaction instead of starting a new one (double charge).
  INPROGRESS no longer stores responseCode 0 (the line counted as paid on the next
  Send); the poll branch resolves the outer promise; not_found resets the
  transaction id; all handlers cope with false/null results from silentCall.
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