Error details for failed receives

[dipunm]

Currently, the library will make some assumptions, and will hide details of what went wrong when a proof fails to be split.

Specifically, I am looking at the following methods:

• https://github.com/cashubtc/cashu-ts/blob/main/src/CashuWallet.ts#L136
• https://github.com/cashubtc/cashu-ts/blob/main/src/CashuWallet.ts#L176
• https://github.com/cashubtc/cashu-ts/blob/main/src/CashuMint.ts#L139

I would like to at the very least be able to identify the difference between a server rejecting a request (token has already been claimed) vs network issues (eg. http timeout or unexpected response)

There are a few ways to achieve this, but with the current design, I am not sure what considerations I am missing. Also with #85 in progress, I think its too early to attempt a PR.

That said, hopefully this can kick off a discussion at the very least.

My initial thoughts here, the least breaking change, is to add a new property to ReceiveTokenEntryResponse and ReceiveResponse to include the original error(s) that caused the failure(s).

That said, I would also suggest a change in design if possible. Returning an object with both proofs or tokens and ____WithErrors implies that you can have partial success. If this is not the case (as I assume is not the case based on the current implementation), then throwing an error seems like a more straightforward and standard solution. If not throwing an error, then an object that more succinctly identifies as an error (eg. a Failed....Response) or just adding a simple flag like success: true|false would help to solidify that there is no partial success.

If there is partial success, I think this becomes a pretty difficult thing to manage, so I hope that's not the case honestly, but if so, then we should explore how clients should handle this and what type of design we need to support that.

[gandlafbtc]

Thanks for opening the issue!

Yes, correct the error handling is very bad right now. That is partialy due to the mints only recent upgrade to expressive errors, and partially due to a switch from axios to fetch in the library that this has not been implemented propperly yet. The mints are upgraded now, and we've switched to fetch, so error handling should be next on the list! Feel free to open a PR regarding this, I will handle the merging.

That said, I would also suggest a change in design if possible

The thing is, we can have partial success because there can be tokens from different mints inside a token. But as far as i can tell from the discussions from the TG channel, we will switch to a simpler data structure that only contains one token per token (duh). is that correct @callebtc ?

this would solve our problem

Proofs inside a token are transactional, if there is a single invalid proof, the token can't be claimed.

[dipunm]

Proofs inside a token are transactional, if there is a single invalid proof, the token can't be claimed.

🔥

[dipunm]

I'll make a pr hopefully tonight and we can build from there 🙏

[dipunm]

Following up on this, I have actually now done 2 stages of refactoring.

The first is in this PR: #88

And I want to get that reviewed and merged before I make the second change in order to keep the changes small and manageable, but I don't mind talking about the change here.

• I looked at the CashuMint.ts file and saw nothing I wanted to refactor there in terms of error handling.
• In CashuWallet.ts however, I was interested in refactoring 2 methods: receiveTokenEntry, and receive.

receiveTokenEntry

By allowing this method to throw errors, we can simplify its return value since there is no partial failure in this method at all. We simply return the same object but without a proofsWithError property.

Before when it failed, it would blindly dump all proofs from the token entry into the list of proofsWithError, log the error message, but fail to give any manageable feedback to the caller.

Now it will throw the error, and since we passed in the original token entry, we already know which proofs failed, we don't lose any information and this method is simpler.

receive

This method is more tricky. Since it works with many tokens, it merely creates an aggregate version of the above method.

Note:
One thing that is confusing, and possibly a bug, is that the way we handle newKeys is not aggregate in fashion. As we iterate through the tokens, the first one to have a newKeys property is what we take. Any tokens after that with a newKeys property will be ignored.

The new return type I have created is as follows:

/**
 * Response when receiving a complete token.
 */
export type ReceiveResponse2 = {
	/**
	 * Successfully received Cashu Token
	 */
	token: Token,
	/**
	 * If the mint has rotated keys, this field will be populated with the new keys.
	 */
	newKeys?: MintKeys,
	/**
	 * TokenEntries that had errors. No error will be thrown, but clients can choose to handle tokens with errors accordingly.
	 */
	failedTokenEntries: {
		tokenEntry: TokenEntry,
		failReason: string
	}[],
}

I wasn't sure whether to include the original Error object, or to distill it into a reason string. This is something I don't mind getting feedback about. We can also consider adding both.

[gandlafbtc]

thanks for the work on this @dipunm . I will close the issue for now. It is being discussed in other places now.