Skip to content
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

[1] Failed to broadcast the transaction: bitshares-crypto digest #342

Closed
abitmore opened this issue Aug 30, 2017 · 9 comments
Closed

[1] Failed to broadcast the transaction: bitshares-crypto digest #342

abitmore opened this issue Aug 30, 2017 · 9 comments
Assignees
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Milestone

Comments

@abitmore
Copy link
Member

We used to be able to know the exact error message when failed to broadcast a transaction from the GUI. But now:

e9077481-2e9b-4cd0-813c-f828d2c958cf

How to reproduce: try to place an order in ICOWT/BTS market pair, if you're not whitelisted.

In the console we can still see the error message:

Error: Assert Exception: is_authorized_asset( d, *_seller, *_receive_asset ): 
bitshares-crypto digest 73af88dc3dad432624af5f67f426be03be5b36ebe735a60f199ba1699d6ccf64 transaction 

But the first line is not shown in GUI.

I'm not sure if it's caused by a node API change. Also reported here: bitshares/bitshares-core#382

@wmbutler wmbutler changed the title Failed to broadcast the transaction: bitshares-crypto digest [1] Failed to broadcast the transaction: bitshares-crypto digest Aug 30, 2017
@wmbutler wmbutler added this to the 170914 milestone Aug 30, 2017
@wmbutler wmbutler added the [3] Feature Classification indicating the addition of novel functionality to the design label Aug 30, 2017
@wmbutler wmbutler changed the title [1] Failed to broadcast the transaction: bitshares-crypto digest [.5] Failed to broadcast the transaction: bitshares-crypto digest Aug 30, 2017
@wmbutler
Copy link
Contributor

Display an appropriate error message for this raw message:

"You are not approved to trade on this market"

I'm open to suggestions on the proper wording for this particular error.

@abitmore
Copy link
Member Author

@wmbutler I'm not talking about a special case, but a common one. Some messages returned by the backend can be shown on UI directly, for example, insufficient balance. Sometimes there are detailed messages, sometimes with error codes. I admit that not all errors come with a message, but it can be fixed in backend.

@wmbutler
Copy link
Contributor

wmbutler commented Aug 30, 2017

Each Message needs a clear plain text message that the end user can understand. Additionally, the user should be able to look at the console for the more technical message. Sounds like we need a crosswalk of json messages to plaintext messages.

@abitmore
Copy link
Member Author

That's why there are error codes. We can have a table, no need to fix every single issue.

@wmbutler
Copy link
Contributor

wmbutler commented Aug 30, 2017

So, you'd like this to be displayed in a location visible to the end user?

Error: Assert Exception: is_authorized_asset( d, *_seller, *_receive_asset ):

And we can leave the crypto-digest message in the console?

@abitmore
Copy link
Member Author

So, you'd like this to be displayed in a location visible to the end user?
Error: Assert Exception: is_authorized_asset( d, *_seller, *_receive_asset ):
And we can leave the crypto-digest message in the console?

Yes, something like this.
Actually in the node(backend), there is usually a message after the last :, so usually GUI need to display that message and do NOT display the expression before the :; sometimes there is also an error code, which can be translated and displayed in GUI; for this special error neither the message nor the code is there, so we should add a message and/or a code in the node (the backend).

@wmbutler wmbutler self-assigned this Aug 31, 2017
@wmbutler
Copy link
Contributor

Bill to review various errors, error codes and plaintext messages and work out a place to display these errors.

@wmbutler wmbutler changed the title [.5] Failed to broadcast the transaction: bitshares-crypto digest [0] Failed to broadcast the transaction: bitshares-crypto digest Aug 31, 2017
@svk31
Copy link
Contributor

svk31 commented Sep 5, 2017

Are there error codes broadcast by bitshares-core? Can't remember seeing any.

I'm not sure what's changed here, as far as I can see the error coming from the API has been treated the same for ever, at least all of 2017, from TransactionConfirmActions.jsx:

// messages of length 1 are local exceptions (use the 1st line)
// longer messages are remote API exceptions (use the 2nd line)
let splitError = error.message.split( "\n" );
let message = splitError[splitError.length === 1 ? 0 : 1];

I can change it to just use the first part of the error, which is the assert exception.

@wmbutler
Copy link
Contributor

wmbutler commented Sep 6, 2017

@abitmore can you clarify for @svk31 ?

@wmbutler wmbutler changed the title [0] Failed to broadcast the transaction: bitshares-crypto digest [1] Failed to broadcast the transaction: bitshares-crypto digest Sep 6, 2017
@wmbutler wmbutler assigned svk31 and unassigned wmbutler Sep 6, 2017
@svk31 svk31 closed this as completed Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Projects
None yet
Development

No branches or pull requests

3 participants