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

Use error codes #717

Open
lukechilds opened this issue Mar 27, 2018 · 2 comments
Open

Use error codes #717

lukechilds opened this issue Mar 27, 2018 · 2 comments

Comments

@lukechilds
Copy link
Contributor

lukechilds commented Mar 27, 2018

Currently, in the case of an error, mm returns an error property in the JSON with the value set as the error message.

This means theres no way to handle the errors without checking the entire string. And if the string ever changes in the future, all GUIs trying to handle that specific error will break because their string check will fail.

A better solution would be to return unique error codes along with each error.

Something like:

{
  error: 123,
  message: "cant find a deposit that is close enough in size."
}

that way people can easily handle that specific error with:

if (response.error === 123) {
	splitUtxos();
}

instead of needing to do:

if (response.error === "cant find a deposit that is close enough in size.") {
	splitUtxos();
}

They can still access your error messages with response.message or supply their own.

And you can update error messages without breaking any GUIs because everyone should be checking based on the codes.

@jl777
Copy link
Owner

jl777 commented Mar 27, 2018

we had making better error handling for 2.0 is it critically required to do now? it seems like a lot of work to find all possible errors and change them, in addition to the existing gui that is relying on the error text

@lukechilds
Copy link
Contributor Author

Related: #594

is it critically required to do now?

No, not critical, just something we ran into and realised our error checks were likely to break in the future if the wording changed. Just wanted to track it here.

DeckerSU pushed a commit to DeckerSU/SuperNET that referenced this issue Nov 5, 2022
…sing reserved gossipsub topic. (jl777#717)

* WIP.

* WIP. Added behaviour that requests known peers information from other peers.

* Maintain known peers list, attempt to dial more peers if connected relayers num < 6.

* Track the peers added our node to their relayers mesh.
Send messages to subscribed peers only if they added us to their relayers mesh.

* Update libp2p, port outbound_substream_establishing state tracking from libp2p upstream.

* Fix clippy.

* WIP. Disconnect the peers if node has excessive number of connections to relayers.

* Refactor. Extract some actions from swarm poll_fn to separate fns.

* Lower mesh_n_low, mesh_n, mesh_n_high for non-relayer nodes.

* Remove not actual comment.

* Avoid long holding of orderbook lock. Spawn request_order in background as it may take long time to run.

* Start announce interval in 60 seconds from now instead of 600.

* Reconnect to bootstrap peers if empty peers are returned from peers exchange.

* Minor fixes. Spawn the entire process_p2p_message in background.
This allows process_p2p_message run as long as required not affecting the main processing loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants