handle error events on xkeys that can be emitted during the init process before the instance is exposed for listener registration #113
Conversation
…ess before the instance is exposed for listener registration
packages/webhid/src/methods.ts
Outdated
| .finally(() => (alreadyRejected = true)) | ||
| } | ||
| // Handle all error events until the instance is returned | ||
| xkeys.on('error', xkeysStopgapErrorHandler) |
There was a problem hiding this comment.
we don't use once intentionally. Until we have resolved back the instance to the caller, we are responsible to keep catching errors that may be emitted.
|
Implementation could be revised with the addition of a |
packages/webhid/src/methods.ts
Outdated
| return | ||
| } | ||
| deviceWrap.close() | ||
| .then(() => reject()) |
There was a problem hiding this comment.
Minor note: is there a reason the reject() has no argument here? Should we reject(e)?
There was a problem hiding this comment.
This is for the path where deviceWrap.close() promise resolves, where it should resolve to a void promise, but still I want to avoid forwarding whatever value it resolves to as the rejected value of the enclosing (returned) promise in case the API of that method changes.
|
I added an implementation for node in #114. I also simplified the implementation here and there to not handle the cleanup inside the promise executor, and instead rely on the existing catch clause which execution will break out to if the promise rejects. |
packages/webhid/src/methods.ts
Outdated
| .close() | ||
| .then(() => reject()) | ||
| .catch(reject) | ||
| .finally(() => (alreadyRejected = true)) |
There was a problem hiding this comment.
doing this in the finally (old implementation) was not semantically coherent since the flag only makes sense when the promise rejects, not when it resolves (same applies below). So changed this.
Fixes #112 (implements the first approach discussed)