Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Register onError handler with Deposit autoSubmit function #119

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions src/Deposit.js
Original file line number Diff line number Diff line change
Expand Up @@ -601,16 +601,6 @@ export default class Deposit {
* @param {Error} err Error
*/

/**
* Registers a handler for errors.
*
* @param {OnErrorHandler} onErrorHandler
* A handler that receives an error.
*/
onError(onErrorHandler) {
this.eventEmitter.on("error", onErrorHandler)
}

// /---------------------------- Event Handlers -----------------------------

/**
Expand Down Expand Up @@ -1014,16 +1004,23 @@ export default class Deposit {
* so a deposit cannot start auto-submitting and then switch to auto-minting
* mid-stream---whichever is called first will be the flow that will occur.
*
* @param {OnErrorHandler} [onError] A handler that receives an error (optional).
*
* @return {AutoSubmitState} An object with promises to various stages of
* the auto-submit lifetime. Each promise can be fulfilled or
* rejected, and they are in a sequence where later promises will be
* rejected by earlier ones.
*/
autoSubmit() {
autoSubmit(onError) {
// Only enable auto-submitting once.
if (this.autoSubmittingState) {
return this.autoSubmittingState
}

if (onError) {
this.eventEmitter.on("error", onError)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't what I'd call the expected behavior, fwiw, because this keeps the error handler registered. Passing it to the function implies a single-shot usage. You could use .one here, but if there's no error invoked later on then that could lead to an unexpected call with a future error event.

I would just invoke this in the same place we trigger the event below, if it's defined.


/** @type {AutoSubmitState} */
this.autoSubmittingState = {
fundingTransaction: this.fundingTransaction,
Expand Down