-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log unknown swap errors #623
Conversation
625859d
to
898fa54
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small cleanups, but what's here would work.
src/core/swap/swap-api.ts
Outdated
function isUnknownSwapError(error: unknown): boolean { | ||
return ( | ||
asMaybeInsufficientFundsError(error) == null && | ||
asMaybePendingFundsError(error) == null && | ||
asMaybeSwapAboveLimitError(error) == null && | ||
asMaybeSwapBelowLimitError(error) == null && | ||
asMaybeSwapPermissionError(error) == null && | ||
asMaybeSwapCurrencyError(error) == null | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of building this whole new function, could we use rankError(error) <= 1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hmm.. The functions are semantically different and their is motivation for them to diverge in the future as we filter out noisy errors. Shall we opt to keep them distinct for this reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we give the error a score, then we know what it is, and by definition the error cannot be "unknown". Let's leverage this fact, but still keep the functions distinct:
function isUnknownSwapError(error: unknown): boolean {
isKnownError = rankError(error) > 1
return !isKnownError
}
src/core/swap/swap-api.ts
Outdated
// Stringify to include "null" | ||
toToken: String(request.toTokenId), | ||
toWalletType: request.toWallet.type, | ||
// toWalletType: request.fromWallet.type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover comment?
898fa54
to
0fefaa4
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none