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

RPC tooBusy response now has 503 HTTP status code: #4143

Merged
merged 1 commit into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions src/ripple/net/impl/RPCCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1396,16 +1396,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
26 changes: 23 additions & 3 deletions src/ripple/protocol/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,26 +163,42 @@ 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.
constexpr ErrorInfo()
: code(rpcUNKNOWN), token("unknown"), message("An unknown error code.")
: code(rpcUNKNOWN)
, token("unknown")
, message("An unknown error code.")
, http_status(200)
{
}

constexpr ErrorInfo(
error_code_i code_,
char const* token_,
char const* message_)
: code(code_), token(token_), message(message_)
: code(code_), token(token_), message(message_), http_status(200)
{
}

constexpr ErrorInfo(
error_code_i code_,
char const* token_,
char const* message_,
int http_status_)
: code(code_)
, token(token_)
, message(message_)
, http_status(http_status_)
{
}

error_code_i code;
Json::StaticString token;
Json::StaticString message;
int http_status;
};

/** Returns an ErrorInfo that reflects the error code. */
Expand Down Expand Up @@ -332,6 +348,10 @@ not_validator_error()
bool
contains_error(Json::Value const& json);

/** Returns http status that corresponds to the error code. */
int
error_code_http_status(error_code_i code);

} // namespace RPC

/** Returns a single string with the contents of an RPC error. */
Expand Down
180 changes: 88 additions & 92 deletions src/ripple/protocol/impl/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
//==============================================================================

#include <ripple/protocol/ErrorCodes.h>
#include <array>
#include <cassert>
#include <stdexcept>

Expand All @@ -26,105 +27,96 @@ namespace RPC {

namespace detail {

// clang-format off
// Unordered array of ErrorInfos, so we don't have to maintain the list
// ordering by hand.
//
// 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.
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."},
{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_ISSUER, "badIssuer", "Issuer account malformed."},
{rpcBAD_MARKET, "badMarket", "No such market."},
{rpcBAD_SECRET, "badSecret", "Secret does not match account."},
{rpcBAD_SEED, "badSeed", "Disallowed seed."},
{rpcBAD_SYNTAX, "badSyntax", "Syntax error."},
{rpcCHANNEL_MALFORMED, "channelMalformed", "Payment channel is malformed."},
{rpcCHANNEL_AMT_MALFORMED, "channelAmtMalformed", "Payment channel amount is malformed."},
{rpcCOMMAND_MISSING, "commandMissing", "Missing command entry."},
{rpcDB_DESERIALIZATION, "dbDeserialization", "Database deserialization error."},
{rpcDST_ACT_MALFORMED, "dstActMalformed", "Destination account is malformed."},
{rpcDST_ACT_MISSING, "dstActMissing", "Destination account not provided."},
{rpcDST_ACT_NOT_FOUND, "dstActNotFound", "Destination account not found."},
{rpcDST_AMT_MALFORMED, "dstAmtMalformed", "Destination amount/currency/issuer is malformed."},
{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"},
{rpcHIGH_FEE, "highFee", "Current transaction fee exceeds your limit."},
{rpcINTERNAL, "internal", "Internal error."},
{rpcINVALID_LGR_RANGE, "invalidLgrRange", "Ledger range is invalid."},
{rpcINVALID_PARAMS, "invalidParams", "Invalid parameters."},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error."},
{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."},
{rpcMASTER_DISABLED, "masterDisabled", "Master key is disabled."},
{rpcNOT_ENABLED, "notEnabled", "Not enabled in configuration."},
{rpcNOT_IMPL, "notImpl", "Not implemented."},
{rpcNOT_READY, "notReady", "Not ready to handle this request."},
{rpcNOT_SUPPORTED, "notSupported", "Operation not supported."},
{rpcNO_CLOSED, "noClosed", "Closed ledger is unavailable."},
{rpcNO_CURRENT, "noCurrent", "Current ledger is unavailable."},
{rpcNOT_SYNCED, "notSynced", "Not synced to the network."},
{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_PF_REQUEST, "noPathRequest", "No pathfinding request in progress."},
{rpcPUBLIC_MALFORMED, "publicMalformed", "Public key is malformed."},
{rpcREPORTING_UNSUPPORTED, "reportingUnsupported", "Requested operation not supported by reporting mode server"},
{rpcSIGNING_MALFORMED, "signingMalformed", "Signing of transaction is malformed."},
{rpcSLOW_DOWN, "slowDown", "You are placing too much load on the server."},
{rpcSRC_ACT_MALFORMED, "srcActMalformed", "Source account is malformed."},
{rpcSRC_ACT_MISSING, "srcActMissing", "Source account not provided."},
{rpcSRC_ACT_NOT_FOUND, "srcActNotFound", "Source account not found."},
{rpcSRC_CUR_MALFORMED, "srcCurMalformed", "Source currency is malformed."},
{rpcSRC_ISR_MALFORMED, "srcIsrMalformed", "Source issuer is malformed."},
{rpcSTREAM_MALFORMED, "malformedStream", "Stream malformed."},
{rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now."},
{rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found."},
{rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method."},
{rpcSENDMAX_MALFORMED, "sendMaxMalformed", "SendMax amount malformed."},
{rpcOBJECT_NOT_FOUND, "objectNotFound", "The requested object was not found."}};
// clang-format on

// C++ does not allow you to return an array from a function. You must
// return an object which may in turn contain an array. The following
// struct is simply defined so the enclosed array can be returned from a
// constexpr function.
//
// In C++17 this struct can be replaced by a std::array. But in C++14
// the constexpr methods of a std::array are not sufficient to perform the
// necessary work at compile time.
template <int N>
struct ErrorInfoArray
{
// Visual Studio doesn't treat a templated aggregate as an aggregate.
// So, for Visual Studio, we define a constexpr default constructor.
constexpr ErrorInfoArray() : infos{}
{
}
// 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.

ErrorInfo infos[N];
};
// 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."},
Comment on lines +48 to +51
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all four of these sound like cases that should be 400 Bad Request

{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.", 400},
{rpcBAD_KEY_TYPE, "badKeyType", "Bad key type.", 400},
{rpcBAD_FEATURE, "badFeature", "Feature unknown or invalid.", 500},
{rpcBAD_ISSUER, "badIssuer", "Issuer account malformed.", 400},
{rpcBAD_MARKET, "badMarket", "No such market.", 404},
{rpcBAD_SECRET, "badSecret", "Secret does not match account.", 403},
{rpcBAD_SEED, "badSeed", "Disallowed seed.", 403},
{rpcBAD_SYNTAX, "badSyntax", "Syntax error.", 400},
{rpcCHANNEL_MALFORMED, "channelMalformed", "Payment channel is malformed.", 400},
{rpcCHANNEL_AMT_MALFORMED, "channelAmtMalformed", "Payment channel amount is malformed.", 400},
{rpcCOMMAND_MISSING, "commandMissing", "Missing command entry.", 400},
{rpcDB_DESERIALIZATION, "dbDeserialization", "Database deserialization error.", 502},
{rpcDST_ACT_MALFORMED, "dstActMalformed", "Destination account is malformed.", 400},
{rpcDST_ACT_MISSING, "dstActMissing", "Destination account not provided.", 400},
{rpcDST_ACT_NOT_FOUND, "dstActNotFound", "Destination account not found.", 404},
{rpcDST_AMT_MALFORMED, "dstAmtMalformed", "Destination amount/currency/issuer is malformed.", 400},
{rpcDST_AMT_MISSING, "dstAmtMissing", "Destination amount/currency/issuer is missing.", 400},
{rpcDST_ISR_MALFORMED, "dstIsrMalformed", "Destination issuer is malformed.", 400},
{rpcEXCESSIVE_LGR_RANGE, "excessiveLgrRange", "Ledger range exceeds 1000.", 400},
{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.", 402},
{rpcINTERNAL, "internal", "Internal error.", 500},
{rpcINVALID_LGR_RANGE, "invalidLgrRange", "Ledger range is invalid.", 400},
{rpcINVALID_PARAMS, "invalidParams", "Invalid parameters.", 400},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error.", 500},
{rpcLGR_IDXS_INVALID, "lgrIdxsInvalid", "Ledger indexes invalid.", 400},
{rpcLGR_IDX_MALFORMED, "lgrIdxMalformed", "Ledger index malformed.", 400},
{rpcLGR_NOT_FOUND, "lgrNotFound", "Ledger not found.", 404},
{rpcLGR_NOT_VALIDATED, "lgrNotValidated", "Ledger not validated.", 202},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure 202 is correct here? That's a "successful" class code so I wouldn't expect an error

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Google...

202 Accepted response status code indicates that the request has been accepted for processing, but the processing has not been completed; in fact, processing may not have started yet.

Yeah, I'm not completely sold on 202 myself. If you get back lgrNotValidated, then the request has been rejected. You could try the same request later, but it would be handled as a brand new request.

{rpcMASTER_DISABLED, "masterDisabled", "Master key is disabled.", 403},
{rpcNOT_ENABLED, "notEnabled", "Not enabled in configuration.", 501},
{rpcNOT_IMPL, "notImpl", "Not implemented.", 501},
{rpcNOT_READY, "notReady", "Not ready to handle this request.", 503},
{rpcNOT_SUPPORTED, "notSupported", "Operation not supported.", 501},
{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.", 405},
{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.", 404},
{rpcOBJECT_NOT_FOUND, "objectNotFound", "The requested object was not found.", 404},
{rpcPUBLIC_MALFORMED, "publicMalformed", "Public key is malformed.", 400},
{rpcREPORTING_UNSUPPORTED, "reportingUnsupported", "Requested operation not supported by reporting mode server", 405},
{rpcSENDMAX_MALFORMED, "sendMaxMalformed", "SendMax amount malformed.", 400},
{rpcSIGNING_MALFORMED, "signingMalformed", "Signing of transaction is malformed.", 400},
{rpcSLOW_DOWN, "slowDown", "You are placing too much load on the server.", 429},
{rpcSRC_ACT_MALFORMED, "srcActMalformed", "Source account is malformed.", 400},
{rpcSRC_ACT_MISSING, "srcActMissing", "Source account not provided.", 400},
{rpcSRC_ACT_NOT_FOUND, "srcActNotFound", "Source account not found.", 404},
{rpcSRC_CUR_MALFORMED, "srcCurMalformed", "Source currency is malformed.", 400},
{rpcSRC_ISR_MALFORMED, "srcIsrMalformed", "Source issuer is malformed.", 400},
{rpcSTREAM_MALFORMED, "malformedStream", "Stream malformed.", 400},
{rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now.", 503},
{rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found.", 404},
{rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method.", 405}};
// clang-format on

// Sort and validate unorderedErrorInfos at compile time. Should be
// converted to consteval when get to C++20.
template <int M, int N>
constexpr auto
sortErrorInfos(ErrorInfo const (&unordered)[N]) -> ErrorInfoArray<M>
sortErrorInfos(ErrorInfo const (&unordered)[N]) -> std::array<ErrorInfo, M>
{
ErrorInfoArray<M> ret;
std::array<ErrorInfo, M> ret = {};

for (ErrorInfo const& info : unordered)
{
Expand All @@ -135,12 +127,10 @@ sortErrorInfos(ErrorInfo const (&unordered)[N]) -> ErrorInfoArray<M>
static_assert(rpcSUCCESS == 0, "Unexpected error_code_i layout.");
int const index{info.code - 1};

if (ret.infos[index].code != rpcUNKNOWN)
if (ret[index].code != rpcUNKNOWN)
throw(std::invalid_argument("Duplicate error_code_i in list"));

ret.infos[index].code = info.code;
ret.infos[index].token = info.token;
ret.infos[index].message = info.message;
ret[index] = info;
}

// Verify that all entries are filled in starting with 1 and proceeding
Expand All @@ -150,7 +140,7 @@ sortErrorInfos(ErrorInfo const (&unordered)[N]) -> ErrorInfoArray<M>
// rpcUNKNOWN. But other than that all entries should match their index.
int codeCount{0};
int expect{rpcBAD_SYNTAX - 1};
for (ErrorInfo const& info : ret.infos)
for (ErrorInfo const& info : ret)
{
++expect;
if (info.code == rpcUNKNOWN)
Expand Down Expand Up @@ -181,7 +171,7 @@ get_error_info(error_code_i code)
{
if (code <= rpcSUCCESS || code > rpcLAST)
return detail::unknownError;
return detail::sortedErrorInfos.infos[code - 1];
return detail::sortedErrorInfos[code - 1];
}

Json::Value
Expand All @@ -208,6 +198,12 @@ contains_error(Json::Value const& json)
return false;
}

int
error_code_http_status(error_code_i code)
{
return get_error_info(code).http_status;
}

} // namespace RPC

std::string
Expand Down
26 changes: 25 additions & 1 deletion src/ripple/rpc/impl/ServerHandlerImp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <ripple/json/to_string.h>
#include <ripple/net/RPCErr.h>
#include <ripple/overlay/Overlay.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/resource/Fees.h>
#include <ripple/resource/ResourceManager.h>
#include <ripple/rpc/RPCHandler.h>
Expand Down Expand Up @@ -970,6 +971,29 @@ ServerHandlerImp::processRequest(
}
}
}

// If we're returning an error_code, use that to determine the HTTP status.
int const httpStatus = [&reply]() {
// This feature is enabled with ripplerpc version 3.0 and above.
// Before ripplerpc version 3.0 always return 200.
if (reply.isMember(jss::ripplerpc) &&
reply[jss::ripplerpc].isString() &&
reply[jss::ripplerpc].asString() >= "3.0")
{
// If there's an error_code, use that to determine the HTTP Status.
if (reply.isMember(jss::error) &&
reply[jss::error].isMember(jss::error_code) &&
reply[jss::error][jss::error_code].isInt())
{
int const errCode = reply[jss::error][jss::error_code].asInt();
return RPC::error_code_http_status(
static_cast<error_code_i>(errCode));
}
}
// Return OK.
return 200;
}();

auto response = to_string(reply);

rpc_time_.notify(std::chrono::duration_cast<std::chrono::milliseconds>(
Expand All @@ -988,7 +1012,7 @@ ServerHandlerImp::processRequest(
stream << "Reply: " << response.substr(0, maxSize);
}

HTTPReply(200, response, output, rpcJ);
HTTPReply(httpStatus, response, output, rpcJ);
}

//------------------------------------------------------------------------------
Expand Down
Loading