Skip to content

Commit

Permalink
[FOLD] Additional HTTP status codes suggested during review
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottschurr
scottschurr committed Aug 4, 2022
1 parent af6649a commit 96a40b1
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 28 deletions.
11 changes: 1 addition & 10 deletions src/ripple/net/impl/RPCCall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,16 +1399,7 @@ struct RPCCallImp
// callbackFuncP.

// Receive reply
if (iStatus == 401)
Throw<std::runtime_error>(
"incorrect rpcuser or rpcpassword (authorization failed)");
else if (
(iStatus >= 400) && (iStatus != 400) && (iStatus != 404) &&
(iStatus != 500)) // ?
Throw<std::runtime_error>(
std::string("server returned HTTP error ") +
std::to_string(iStatus));
else if (strData.empty())
if (strData.empty())
Throw<std::runtime_error>("no response from server");

// Parse reply
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/ErrorCodes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ enum warning_code_i {

namespace RPC {

/** Maps an rpc error code to its token and default message. */
/** Maps an rpc error code to its token, default message, and HTTP status. */
struct ErrorInfo
{
// Default ctor needed to produce an empty std::array during constexpr eval.
Expand Down
40 changes: 25 additions & 15 deletions src/ripple/protocol/impl/ErrorCodes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,28 @@ namespace detail {
//
// This array will be omitted from the object file; only the sorted version
// will remain in the object file. But the string literals will remain.
//
// There's a certain amount of tension in determining the correct HTTP
// status to associate with a given RPC error. Initially all RPC errors
// returned 200 (OK). And that's the default behavior if no HTTP status code
// is specified below.
//
// The codes currently selected target the load balancer fail-over use case.
// If a query fails on one node but is likely to have a positive outcome
// on a different node, then the failure should return a 4xx/5xx range
// status code.

// clang-format off
constexpr static ErrorInfo unorderedErrorInfos[]{
{rpcACT_MALFORMED, "actMalformed", "Account malformed."},
{rpcACT_NOT_FOUND, "actNotFound", "Account not found."},
{rpcALREADY_MULTISIG, "alreadyMultisig", "Already multisigned."},
{rpcALREADY_SINGLE_SIG, "alreadySingleSig", "Already single-signed."},
{rpcAMENDMENT_BLOCKED, "amendmentBlocked", "Amendment blocked, need upgrade."},
{rpcEXPIRED_VALIDATOR_LIST, "unlBlocked", "Validator list expired."},
{rpcAMENDMENT_BLOCKED, "amendmentBlocked", "Amendment blocked, need upgrade.", 503},
{rpcEXPIRED_VALIDATOR_LIST, "unlBlocked", "Validator list expired.", 503},
{rpcATX_DEPRECATED, "deprecated", "Use the new API or specify a ledger range."},
{rpcBAD_KEY_TYPE, "badKeyType", "Bad key type."},
{rpcBAD_FEATURE, "badFeature", "Feature unknown or invalid."},
{rpcBAD_FEATURE, "badFeature", "Feature unknown or invalid.", 500},
{rpcBAD_ISSUER, "badIssuer", "Issuer account malformed."},
{rpcBAD_MARKET, "badMarket", "No such market."},
{rpcBAD_SECRET, "badSecret", "Secret does not match account."},
Expand All @@ -60,28 +70,28 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
{rpcDST_AMT_MISSING, "dstAmtMissing", "Destination amount/currency/issuer is missing."},
{rpcDST_ISR_MALFORMED, "dstIsrMalformed", "Destination issuer is malformed."},
{rpcEXCESSIVE_LGR_RANGE, "excessiveLgrRange", "Ledger range exceeds 1000."},
{rpcFORBIDDEN, "forbidden", "Bad credentials."},
{rpcFAILED_TO_FORWARD, "failedToForward", "Failed to forward request to p2p node"},
{rpcFORBIDDEN, "forbidden", "Bad credentials.", 403},
{rpcFAILED_TO_FORWARD, "failedToForward", "Failed to forward request to p2p node", 503},
{rpcHIGH_FEE, "highFee", "Current transaction fee exceeds your limit."},
{rpcINTERNAL, "internal", "Internal error.", 500},
{rpcINVALID_LGR_RANGE, "invalidLgrRange", "Ledger range is invalid."},
{rpcINVALID_PARAMS, "invalidParams", "Invalid parameters.", 400},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error."},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error.", 500},
{rpcLGR_IDXS_INVALID, "lgrIdxsInvalid", "Ledger indexes invalid."},
{rpcLGR_IDX_MALFORMED, "lgrIdxMalformed", "Ledger index malformed."},
{rpcLGR_NOT_FOUND, "lgrNotFound", "Ledger not found."},
{rpcLGR_NOT_VALIDATED, "lgrNotValidated", "Ledger not validated."},
{rpcLGR_NOT_VALIDATED, "lgrNotValidated", "Ledger not validated.", 202},
{rpcMASTER_DISABLED, "masterDisabled", "Master key is disabled."},
{rpcNOT_ENABLED, "notEnabled", "Not enabled in configuration."},
{rpcNOT_ENABLED, "notEnabled", "Not enabled in configuration.", 501},
{rpcNOT_IMPL, "notImpl", "Not implemented.", 501},
{rpcNOT_READY, "notReady", "Not ready to handle this request."},
{rpcNOT_READY, "notReady", "Not ready to handle this request.", 202},
{rpcNOT_SUPPORTED, "notSupported", "Operation not supported.", 501},
{rpcNO_CLOSED, "noClosed", "Closed ledger is unavailable."},
{rpcNO_CURRENT, "noCurrent", "Current ledger is unavailable."},
{rpcNOT_SYNCED, "notSynced", "Not synced to the network."},
{rpcNO_CLOSED, "noClosed", "Closed ledger is unavailable.", 503},
{rpcNO_CURRENT, "noCurrent", "Current ledger is unavailable.", 503},
{rpcNOT_SYNCED, "notSynced", "Not synced to the network.", 503},
{rpcNO_EVENTS, "noEvents", "Current transport does not support events."},
{rpcNO_NETWORK, "noNetwork", "Not synced to the network."},
{rpcNO_PERMISSION, "noPermission", "You don't have permission for this command."},
{rpcNO_NETWORK, "noNetwork", "Not synced to the network.", 503},
{rpcNO_PERMISSION, "noPermission", "You don't have permission for this command.", 401},
{rpcNO_PF_REQUEST, "noPathRequest", "No pathfinding request in progress."},
{rpcOBJECT_NOT_FOUND, "objectNotFound", "The requested object was not found."},
{rpcPUBLIC_MALFORMED, "publicMalformed", "Public key is malformed."},
Expand All @@ -97,7 +107,7 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
{rpcSTREAM_MALFORMED, "malformedStream", "Stream malformed."},
{rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now.", 503},
{rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found."},
{rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method."}};
{rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method.", 405}};
// clang-format on

// Sort and validate unorderedErrorInfos at compile time. Should be
Expand Down
17 changes: 16 additions & 1 deletion src/ripple/server/impl/JSONRPCUtil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ HTTPReply(
{
JLOG(j.trace()) << "HTTP Reply " << nStatus << " " << content;

if (nStatus == 401)
if (content.empty() && nStatus == 401)
{
output("HTTP/1.0 401 Authorization Required\r\n");
output(getHTTPHeaderTimestamp());
Expand Down Expand Up @@ -100,18 +100,33 @@ HTTPReply(
case 200:
output("HTTP/1.1 200 OK\r\n");
break;
case 202:
output("HTTP/1.1 202 Accepted\r\n");
break;
case 400:
output("HTTP/1.1 400 Bad Request\r\n");
break;
case 401:
output("HTTP/1.1 401 Authorization Required\r\n");
break;
case 403:
output("HTTP/1.1 403 Forbidden\r\n");
break;
case 404:
output("HTTP/1.1 404 Not Found\r\n");
break;
case 405:
output("HTTP/1.1 405 Method Not Allowed\r\n");
break;
case 429:
output("HTTP/1.1 429 Too Many Requests\r\n");
break;
case 500:
output("HTTP/1.1 500 Internal Server Error\r\n");
break;
case 501:
output("HTTP/1.1 501 Not Implemented\r\n");
break;
case 503:
output("HTTP/1.1 503 Server is overloaded\r\n");
break;
Expand Down
2 changes: 1 addition & 1 deletion src/test/rpc/LedgerRPC_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,7 @@ class LedgerRPC_test : public beast::unit_test::suite
void
testLedgerAccountsOption()
{
testcase("Ledger Request, Accounts Option");
testcase("Ledger Request, Accounts Hashes");
using namespace test::jtx;

Env env{*this};
Expand Down

0 comments on commit 96a40b1

Please sign in to comment.