fix(android): reject the promise on SDK errors instead of crashing the app

[tolypash]

The bug

pairTerminal, charge and refund all do this:

} catch (e: Exception) {
    promise.reject(e.toCodedException())
    throw e                              // ← escapes the module
}

Rejecting is correct. The rethrow then sends the same exception out of the module's coroutine — and for refund, which is .runOnQueue(Queues.MAIN), out on the main queue. So an error the JS side is fully equipped to handle instead takes the process down.

Seen in production (Sally POS, Yoco terminals):

• on a device where YocoSDK.initialise() had failed (its Koin graph couldn't build BaseURLsConfigProvider), every later SDK call threw — pressing Void killed the app instead of showing "could not start refund";
• a charge that rounded to amountInCents: 0 hit IllegalArgumentException: Amount provided needs to be a positive number and did the same.

Both are ordinary, recoverable errors on the JS side. Neither should be fatal.

The change

• drop the throw e in the three AsyncFunction catch blocks — the promise rejection is the error channel;
• null the stored promise on the failure path. chargePromise / refundPromise / pairTerminalPromise are otherwise only cleared in the OnActivityResult handlers, so a rejected-but-retained promise could later be resolved by an unrelated activity result, settling it twice (a crash of its own under the New Architecture);
• configure keeps its rethrow — it's a synchronous Function, where throwing is how the error reaches JS. That path is what surfaces the init failure to the app, and it should stay.

Second, unrelated one-liner

ReceiptInfo.transactionTime is missing @Field, so it never serialises into the JS payment result on Android — anything reading receiptInfo.transactionTime silently gets undefined. iOS exposes it. Added the annotation.

Verified against the SDK

Decompiled com.yoco:payment-ui:2.6.9 from the Gradle cache to confirm the surrounding assumptions (Yoco's docs site currently 404s on every developer.yoco.com/in-person/android/* page through its redirect to yoco.docs.buildwithfern.com):

• public void refund(Context, String transactionId, RefundParameters, PrintParameters, ActivityResultLauncher) — the parameter is named transactionId, and the method throws IllegalArgumentException("An empty transaction cannot be refunded") on a blank id. It is not the clientTransactionId; PaymentResult carries both separately, and this module already surfaces both.
• No behavioural change to any success path here — only what happens when a launch throws.

Testing

Not covered by automated tests (the module has none, and this is native error-path behaviour). Reviewed by inspection; the intended result is that a failing charge/refund/pairTerminal arrives in JS as a rejected promise carrying the SDK's message, with the app still running.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved recovery after terminal pairing, charging, or refund failures by resetting the operation state.
  • Prevented failed payment operations from producing duplicate or unexpected error propagation.
  • Receipt details now correctly include the transaction time when displayed or processed.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Android module now resets stored promises when pairTerminal, charge, or refund fails without rethrowing exceptions. ReceiptInfo.transactionTime is also marked as a record field with @Field.

Changes

Payment handling

Layer / File(s)	Summary
Promise error cleanup android/src/main/java/expo/modules/yoco/ReactNativeYocoModule.kt	Failure paths for terminal pairing, charging, and refunds clear stored promise references, reject the caller promise, and no longer rethrow caught exceptions.
Receipt transaction field android/src/main/java/expo/modules/yoco/data/result/ReactNativeYocoPaymentResult.kt	ReceiptInfo.transactionTime now has the @Field annotation.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main Android error-handling change and reflects the primary user-facing impact.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dont-crash-on-sdk-errors

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

android/src/main/java/expo/modules/yoco/ReactNativeYocoModule.kt (1)

75-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression tests for the new failure lifecycle.

Cover each operation with a mocked synchronous SDK exception, asserting that the JS promise rejects, the exception does not escape, and a later activity result cannot settle the cleared promise.

Also applies to: 109-110, 191-192

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@android/src/main/java/expo/modules/yoco/ReactNativeYocoModule.kt` around
lines 75 - 76, Add regression coverage for the failure paths in the affected
Yoco module operations, including the handlers around pairTerminalPromise and
the corresponding lines noted in the comment. Mock synchronous SDK exceptions
and assert each operation rejects its JS promise without propagating the
exception, clears the pending promise, and ignores any later activity result
rather than settling it.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@android/src/main/java/expo/modules/yoco/ReactNativeYocoModule.kt`:
- Around line 75-76: Add regression coverage for the failure paths in the
affected Yoco module operations, including the handlers around
pairTerminalPromise and the corresponding lines noted in the comment. Mock
synchronous SDK exceptions and assert each operation rejects its JS promise
without propagating the exception, clears the pending promise, and ignores any
later activity result rather than settling it.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 339f3ddf-0af8-4e65-bd50-c4b9f5c59444

📥 Commits

Reviewing files that changed from the base of the PR and between 145c890 and 7d9fb79.

📒 Files selected for processing (2)

• android/src/main/java/expo/modules/yoco/ReactNativeYocoModule.kt
• android/src/main/java/expo/modules/yoco/data/result/ReactNativeYocoPaymentResult.kt