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.

Sign up for GitHub

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

Jump to bottom

fix a misleading message during maintenance #10743

Merged
merged 1 commit into from
Dec 19, 2019
Merged

fix a misleading message during maintenance #10743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 11 additions & 12 deletions arangod/GeneralServer/CommTask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ CommTask::Flow CommTask::prepareExecution(GeneralRequest& req) {
LOG_TOPIC("63f47", TRACE, arangodb::Logger::FIXME)
<< "Maintenance mode: refused path: " << path;
addErrorResponse(ResponseCode::SERVICE_UNAVAILABLE, req.contentTypeResponse(),
req.messageId(), TRI_ERROR_FORBIDDEN);
req.messageId(), TRI_ERROR_HTTP_SERVICE_UNAVAILABLE,
"service unavailable due to startup or maintenance mode");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we consider this a breaking change?

return Flow::Abort;
}
break;
Expand Down Expand Up @@ -443,26 +444,25 @@ RequestStatistics* CommTask::stealStatistics(uint64_t id) {
/// @brief send response including error response body

void CommTask::addErrorResponse(rest::ResponseCode code,
rest::ContentType respType, uint64_t messageId,
int errorNum, std::string const& msg) {
rest::ContentType respType, uint64_t messageId,
int errorNum, char const* errorMessage /* = nullptr */) {
if (errorMessage == nullptr) {
errorMessage = TRI_errno_string(errorNum);
}
TRI_ASSERT(errorMessage != nullptr);

VPackBuffer<uint8_t> buffer;
VPackBuilder builder(buffer);
builder.openObject();
builder.add(StaticStrings::Error, VPackValue(errorNum != TRI_ERROR_NO_ERROR));
builder.add(StaticStrings::ErrorNum, VPackValue(errorNum));
builder.add(StaticStrings::ErrorMessage, VPackValue(msg));
builder.add(StaticStrings::Code, VPackValue((int)code));
builder.add(StaticStrings::ErrorMessage, VPackValue(errorMessage));
builder.add(StaticStrings::Code, VPackValue(static_cast<int>(code)));
builder.close();

addSimpleResponse(code, respType, messageId, std::move(buffer));
}


void CommTask::addErrorResponse(rest::ResponseCode code, rest::ContentType respType,
uint64_t messageId, int errorNum) {
addErrorResponse(code, respType, messageId, errorNum, TRI_errno_string(errorNum));
}

// -----------------------------------------------------------------------------
// --SECTION-- private methods
// -----------------------------------------------------------------------------
Expand All @@ -472,7 +472,6 @@ void CommTask::addErrorResponse(rest::ResponseCode code, rest::ContentType respT
// and scheduled later when the number of used threads decreases

bool CommTask::handleRequestSync(std::shared_ptr<RestHandler> handler) {

RequestStatistics::SET_QUEUE_START(handler->statistics(), SchedulerFeature::SCHEDULER->queueStatistics()._queued);

RequestLane lane = handler->getRequestLane();
Expand Down
4 changes: 1 addition & 3 deletions arangod/GeneralServer/CommTask.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,7 @@ class CommTask : public std::enable_shared_from_this<CommTask> {

/// @brief send response including error response body
void addErrorResponse(rest::ResponseCode, rest::ContentType,
uint64_t messageId, int errorNum, std::string const&);
void addErrorResponse(rest::ResponseCode, rest::ContentType,
uint64_t messageId, int errorNum);
uint64_t messageId, int errorNum, char const* errorMessage = nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the perfect use case for a string_view. It can be implicitly constructed (one can argue about that) from string and const char* and you can extract strings from a Slice as view. When explicit ownership is not required it is the most versatile solution.


////////////////////////////////////////////////////////////////////////////////
/// @brief checks the access rights for a specified path, includes automatic
Expand Down