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

Consolidate "Not Synced" Error Messages #3269

Closed
mDuo13 opened this issue Feb 22, 2020 · 2 comments
Closed

Consolidate "Not Synced" Error Messages #3269

mDuo13 opened this issue Feb 22, 2020 · 2 comments
Assignees
Labels
API Change Feature Request Used to indicate requests to add new features

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Feb 22, 2020

Summary

The server has several different types of error messages it may return when you make an API request it cannot serve because it is not fully synced to the network. We should consolidate these into a single error message.

Motivation

There are at least three universal errors that mean basically the same thing: the server isn't connected to the network. These are:

  • noNetwork
  • noCurrent
  • noClosed

On top of that, there's at least one more command-specific error message, lgrIdxsInvalid, which can happen as a result of not being synced. There may be more I don't know about.

The distinctions between these types of errors are not useful for API consumers who just want to know why their request didn't go through. In theory, the differences might have been helpful for debugging, but in practice, the action an API should take after getting basically all of these messages is the same:

Having different error messages for all these similar cases just makes it harder to look up, harder to recognize a familiar situation, and harder to handle automatically as a client.

Solution

We should consolidate all these error messages into a single message, notSynced or something along those lines. If the server is not synced, or is still in the startup phases (even in stand-alone mode) it returns this error instead of any of the above errors.

Paths Not Taken

I considered maybe there should be a different error, such as notStarted, for when you query a stand-alone-mode server too early in the startup process, but decided it didn't seem like a significant enough case to warrant a different error.

@mDuo13 mDuo13 added API Change Feature Request Used to indicate requests to add new features labels Feb 22, 2020
@HowardHinnant
Copy link
Contributor

Do we need to go through a deprecation period where both the old errors and the new error coexist prior to removing the old errors?

@mDuo13
Copy link
Collaborator Author

mDuo13 commented May 14, 2020

I think the existing codes are piecemeal and inconsistent enough in this case that it seems unlikely to actually break someone's code, but I could be wrong about that.

To be safe, I think it might be necessary to bump the API version for this change, which would mean the old API version would use the old codes and the new API version would use the new unified error code.

HowardHinnant added a commit to HowardHinnant/rippled that referenced this issue May 27, 2020
* Fixes XRPLF#3269

* The error noClosed as been renamed to notSynced.
* Uses of the error noCurrent, noNetwork and lgrIdxsInvalid
  now use notSynced.
HowardHinnant added a commit to HowardHinnant/rippled that referenced this issue May 27, 2020
* Fixes XRPLF#3269

* The error noClosed as been renamed to notSynced.
* Uses of the error noCurrent, noNetwork and lgrIdxsInvalid
  now use notSynced.
HowardHinnant added a commit to HowardHinnant/rippled that referenced this issue Jun 15, 2020
* Fixes XRPLF#3269

* The error noClosed as been renamed to notSynced.
* Uses of the error noCurrent, noNetwork and lgrIdxsInvalid
  now use notSynced.
HowardHinnant added a commit to HowardHinnant/rippled that referenced this issue Jun 22, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this issue Jun 24, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
* It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this issue Jun 25, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
* It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this issue Jun 25, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
* It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this issue Jun 25, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
* It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this issue Jun 25, 2020
* Fixes XRPLF#3269

* Work on a version 2 of the XRP Network API has begun.
  The new API returns the code `notSynced` in place of
  `noClosed`, `noCurrent`, and `noNetwork`.  And
  `invalidParams` is returned in place of `lgrIdxInvalid`.
* The version 2 API can be specified by adding
  "api_version" = 2 to your json request.  The default
  version remains 1 (if unspecified), except for the command
  line interface which tracks version 2.
* It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change Feature Request Used to indicate requests to add new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants