Skip to content

Commit

Permalink
Enhance hierarchical addition on exception context (facebookincubator…
Browse files Browse the repository at this point in the history 
…#9695)

Summary:
Pull Request resolved: facebookincubator#9695

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321

fbshipit-source-id: abc56abb72b0e526d8858232031279ad7a72a238
  • Loading branch information
@facebook-github-bot
Bikramjeet Vig authored and facebook-github-bot committed May 6, 2024
1 parent cdf0a70 commit 5bf1e27
Show file tree
Hide file tree
Showing 9 changed files with 317 additions and 96 deletions.
48 changes: 25 additions & 23 deletions velox/common/base/VeloxException.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,31 @@ ExceptionContext& getExceptionContext() {
return context;
}

// Retrieves the message of the top-level ancestor of the current exception
// context. If the top-level context message is not empty and is the same as the
// current one, returns a string indicating they are the same.
std::string getTopLevelExceptionContextString(
// Traverses the context hierarchy and appends messages from all contexts that
// are marked as essential.
std::string getAdditionalExceptionContextString(
VeloxException::Type exceptionType,
const std::string& currentMessage) {
auto* context = &getExceptionContext();
if (context->parent && context->parent->parent) {
while (context->parent && context->parent->parent) {
context = context->parent;
}
auto topLevelMessage = context->message(exceptionType);
if (!topLevelMessage.empty() && topLevelMessage == currentMessage) {
return "Same as context.";
} else {
return topLevelMessage;
std::string additionalMessage = "";
if (!context->parent || !context->parent->parent) {
return additionalMessage;
}
context = context->parent;
while (context->parent) {
if (context->isEssential) {
auto message = context->message(exceptionType);
if (!message.empty()) {
additionalMessage += message + " ";
}
}
context = context->parent;
}

if (!currentMessage.empty()) {
return "Same as context.";
if (!additionalMessage.empty()) {
// Get rid of the extra space at the end.
additionalMessage.pop_back();
}
return "";
return additionalMessage;
}

VeloxException::VeloxException(
Expand All @@ -90,8 +92,8 @@ VeloxException::VeloxException(
state.errorSource = errorSource;
state.errorCode = errorCode;
state.context = getExceptionContext().message(exceptionType);
state.topLevelContext =
getTopLevelExceptionContextString(exceptionType, state.context);
state.additionalContext =
getAdditionalExceptionContextString(exceptionType, state.context);
state.isRetriable = isRetriable;
})) {}

Expand All @@ -114,8 +116,8 @@ VeloxException::VeloxException(
state.errorSource = errorSource;
state.errorCode = errorCode;
state.context = getExceptionContext().message(exceptionType);
state.topLevelContext =
getTopLevelExceptionContextString(exceptionType, state.context);
state.additionalContext =
getAdditionalExceptionContextString(exceptionType, state.context);
state.isRetriable = isRetriable;
state.wrappedException = e;
})) {}
Expand Down Expand Up @@ -223,8 +225,8 @@ void VeloxException::State::finalize() const {
elaborateMessage += "Context: " + context + "\n";
}

if (!topLevelContext.empty()) {
elaborateMessage += "Top-Level Context: " + topLevelContext + "\n";
if (!additionalContext.empty()) {
elaborateMessage += "Additional Context: " + additionalContext + "\n";
}

if (function) {
Expand Down
10 changes: 7 additions & 3 deletions velox/common/base/VeloxException.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ class VeloxException : public std::exception {
return state_->context;
}

const std::string& topLevelContext() const {
return state_->topLevelContext;
const std::string& additionalContext() const {
return state_->additionalContext;
}

const std::exception_ptr& wrappedException() const {
Expand All @@ -230,7 +230,7 @@ class VeloxException : public std::exception {
// The current exception context.
std::string context;
// The top-level ancestor of the current exception context.
std::string topLevelContext;
std::string additionalContext;
bool isRetriable;
// The original std::exception.
std::exception_ptr wrappedException;
Expand Down Expand Up @@ -353,6 +353,10 @@ struct ExceptionContext {
/// Value to pass to `messageFunc`. Can be null.
void* arg{nullptr};

/// If true, then the addition context in 'this' is always included when there
/// are hierarchical exception contexts.
bool isEssential{false};

/// Pointer to the parent context when there are hierarchical exception
/// contexts.
ExceptionContext* parent{nullptr};
Expand Down
180 changes: 165 additions & 15 deletions velox/common/base/tests/ExceptionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,11 +583,13 @@ TEST(ExceptionTest, context) {
};

{
// Create multi-layer contexts.
// Create multi-layer contexts with top level marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter topLevelContext(
{messageFunction, &topLevelTroubleshootingAid});
facebook::velox::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction,
.arg = &topLevelTroubleshootingAid,
.isEssential = true});

MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
Expand All @@ -608,7 +610,7 @@ TEST(ExceptionTest, context) {
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: System error: Inner-level troubleshooting aid."
"\nTop-Level Context: System error: Top-level troubleshooting aid."
"\nAdditional Context: System error: Top-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

Expand All @@ -623,13 +625,164 @@ TEST(ExceptionTest, context) {
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: User error: Inner-level troubleshooting aid."
"\nTop-Level Context: User error: Top-level troubleshooting aid."
"\nAdditional Context: User error: Top-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(4, callCount);
}

{
callCount = 0;
// Create multi-layer contexts with middle level marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction, .arg = &topLevelTroubleshootingAid});

MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter midLevelContext(
{.messageFunc = messageFunction,
.arg = &midLevelTroubleshootingAid,
.isEssential = true});

MessageFunctionArg innerLevelTroubleshootingAid{
"Inner-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter innerLevelContext(
{messageFunction, &innerLevelTroubleshootingAid});

verifyVeloxException(
[&]() { VELOX_CHECK_EQ(1, 3); },
"Exception: VeloxRuntimeError"
"\nError Source: RUNTIME"
"\nError Code: INVALID_STATE"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: System error: Inner-level troubleshooting aid."
"\nAdditional Context: System error: Mid-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(2, callCount);

verifyVeloxException(
[&]() { VELOX_USER_CHECK_EQ(1, 3); },
"Exception: VeloxUserError"
"\nError Source: USER"
"\nError Code: INVALID_ARGUMENT"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: User error: Inner-level troubleshooting aid."
"\nAdditional Context: User error: Mid-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(4, callCount);
}

{
callCount = 0;
// Create multi-layer contexts with none marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction, .arg = &topLevelTroubleshootingAid});

MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter midLevelContext(
{.messageFunc = messageFunction, .arg = &midLevelTroubleshootingAid});

MessageFunctionArg innerLevelTroubleshootingAid{
"Inner-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter innerLevelContext(
{messageFunction, &innerLevelTroubleshootingAid});

verifyVeloxException(
[&]() { VELOX_CHECK_EQ(1, 3); },
"Exception: VeloxRuntimeError"
"\nError Source: RUNTIME"
"\nError Code: INVALID_STATE"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: System error: Inner-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(1, callCount);

verifyVeloxException(
[&]() { VELOX_USER_CHECK_EQ(1, 3); },
"Exception: VeloxUserError"
"\nError Source: USER"
"\nError Code: INVALID_ARGUMENT"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: User error: Inner-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(2, callCount);
}

{
callCount = 0;
// Create multi-layer contexts with all ancestors marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction,
.arg = &topLevelTroubleshootingAid,
.isEssential = true});

MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter midLevelContext(
{.messageFunc = messageFunction,
.arg = &midLevelTroubleshootingAid,
.isEssential = true});

MessageFunctionArg innerLevelTroubleshootingAid{
"Inner-level troubleshooting aid.", &callCount};
facebook::velox::ExceptionContextSetter innerLevelContext(
{messageFunction, &innerLevelTroubleshootingAid});

verifyVeloxException(
[&]() { VELOX_CHECK_EQ(1, 3); },
"Exception: VeloxRuntimeError"
"\nError Source: RUNTIME"
"\nError Code: INVALID_STATE"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: System error: Inner-level troubleshooting aid."
"\nAdditional Context: System error: Mid-level troubleshooting aid. System error: Top-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(3, callCount);

verifyVeloxException(
[&]() { VELOX_USER_CHECK_EQ(1, 3); },
"Exception: VeloxUserError"
"\nError Source: USER"
"\nError Code: INVALID_ARGUMENT"
"\nReason: (1 vs. 3)"
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: User error: Inner-level troubleshooting aid."
"\nAdditional Context: User error: Mid-level troubleshooting aid. User error: Top-level troubleshooting aid."
"\nFunction: operator()"
"\nFile: ");

EXPECT_EQ(6, callCount);
}

// Different context.
{
callCount = 0;
Expand All @@ -649,7 +802,6 @@ TEST(ExceptionTest, context) {
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: System error: Debugging info."
"\nTop-Level Context: Same as context."
"\nFunction: operator()"
"\nFile: ");

Expand All @@ -664,7 +816,6 @@ TEST(ExceptionTest, context) {
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: User error: Debugging info."
"\nTop-Level Context: Same as context."
"\nFunction: operator()"
"\nFile: ");

Expand Down Expand Up @@ -709,7 +860,6 @@ TEST(ExceptionTest, context) {
"\nRetriable: False"
"\nExpression: 1 == 3"
"\nContext: Failed to produce additional context."
"\nTop-Level Context: Same as context."
"\nFunction: operator()"
"\nFile: ");

Expand Down Expand Up @@ -743,7 +893,7 @@ TEST(ExceptionTest, wrappedException) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_TRUE(ve.isUserError());
ASSERT_EQ(ve.context(), "");
ASSERT_EQ(ve.topLevelContext(), "");
ASSERT_EQ(ve.additionalContext(), "");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand All @@ -755,7 +905,7 @@ TEST(ExceptionTest, wrappedException) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_FALSE(ve.isUserError());
ASSERT_EQ(ve.context(), "");
ASSERT_EQ(ve.topLevelContext(), "");
ASSERT_EQ(ve.additionalContext(), "");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand Down Expand Up @@ -784,7 +934,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {

std::string data = "lakes";
facebook::velox::ExceptionContextSetter context(
{messageFunction, data.data()});
{messageFunction, data.data(), true});

try {
throw std::invalid_argument("This is a test.");
Expand All @@ -793,7 +943,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_TRUE(ve.isUserError());
ASSERT_EQ(ve.context(), "User error: lakes");
ASSERT_EQ(ve.topLevelContext(), "Same as context.");
ASSERT_EQ(ve.additionalContext(), "");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand All @@ -805,7 +955,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_FALSE(ve.isUserError());
ASSERT_EQ(ve.context(), "System error: lakes");
ASSERT_EQ(ve.topLevelContext(), "Same as context.");
ASSERT_EQ(ve.additionalContext(), "");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand All @@ -821,7 +971,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_TRUE(ve.isUserError());
ASSERT_EQ(ve.context(), "User error: mountains");
ASSERT_EQ(ve.topLevelContext(), "User error: lakes");
ASSERT_EQ(ve.additionalContext(), "User error: lakes");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand All @@ -833,7 +983,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {
ASSERT_EQ(ve.message(), "This is a test.");
ASSERT_FALSE(ve.isUserError());
ASSERT_EQ(ve.context(), "System error: mountains");
ASSERT_EQ(ve.topLevelContext(), "System error: lakes");
ASSERT_EQ(ve.additionalContext(), "System error: lakes");
ASSERT_THROW(
std::rethrow_exception(ve.wrappedException()), std::invalid_argument);
}
Expand Down
Loading

0 comments on commit 5bf1e27

Please sign in to comment.