Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
MXS-4783: Allow exceptions from all API functions
The clientReply and handleError API functions can now also throw
exceptions.

If clientReply throws an exception, the session will be closed; this is
the same as returning false would do. The return value in clientReply is
largely useless as routing replies doesn't seem to have any error
conditions in the current codebase.

If handleError throws an exception, it is propagated upwards to the
parent component. In case of a single-level service, this is the
MXS_SESSION which will simply kill the session.

Documented the exception behaviors of all API functions into the
relevant locations and added extra comments to clarify why calls to
mxs::Component::handleError cannot throw but calls to
mxs::Routable::handleError may throw.
  • Loading branch information
markus456 committed Jan 3, 2024
1 parent 2f0fe40 commit cfce684
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 7 deletions.
11 changes: 9 additions & 2 deletions include/maxscale/filter.hh
Expand Up @@ -141,7 +141,11 @@ public:
*
* @param pPacket A client packet.
*
* @return True for success, false for error
* @return True for success, false for error. If the function return false, a call to the parent
* component's handleError method is made.
*
* @throws May throw mxb::Exception to abort the routing. If the exception is thrown, a call to the parent
* component's handleError method is made.
*/
bool routeQuery(GWBUF&& packet) override;

Expand All @@ -153,7 +157,10 @@ public:
* @param down The downstream components where the response came from
* @param reply The reply information (@see target.hh)
*
* @return True for success, false for error
* @return True for success, false for error. If the function returns false, the session will be closed.
*
* @throws May throw mxb::Exception. If the exception is thrown, the exception message is logged and the
* session will be closed.
*/
bool clientReply(GWBUF&& packet, const mxs::ReplyRoute& down, const mxs::Reply& reply) override;

Expand Down
11 changes: 10 additions & 1 deletion include/maxscale/router.hh
Expand Up @@ -62,6 +62,11 @@ public:
* @param pPacket A buffer containing the reply from the backend
* @param down The route the reply took
* @param reply The reply object
*
* @return True for success, false for error. If the function returns false, the session will be closed.
*
* @throws May throw mxb::Exception. If the exception is thrown, the exception message is logged and the
* session will be closed.
*/
bool clientReply(GWBUF&& packet, const mxs::ReplyRoute& down, const mxs::Reply& reply) override;

Expand All @@ -77,7 +82,11 @@ public:
* @param pProblem The DCB on which the error occurred.
* @param reply The reply object for this endpoint.
*
* @return True if the session can continue, false if the session should be closed
* @return True if the session can continue, false if the session should be closed. If the function
* returns false, the session will be closed.
*
* @throws May throw mxb::Exception. If the exception is thrown, a call to the parent
* component's handleError method is made to proparage the error upwards.
*/
virtual bool
handleError(mxs::ErrorType type, const std::string& message, mxs::Endpoint* pProblem,
Expand Down
13 changes: 11 additions & 2 deletions include/maxscale/routing.hh
Expand Up @@ -35,7 +35,12 @@ public:
* Called when a packet is traveling downstream, towards a backend.
*
* @param pPacket Packet to route
* @return True for success, false for error
*
* @return True for success, false for error. If the function return false, a call to the parent
* component's handleError method is made.
*
* @throws May throw mxb::Exception to abort the routing. If the exception is thrown, a call to the parent
* component's handleError method is made.
*/
virtual bool routeQuery(GWBUF&& packet) = 0;

Expand All @@ -45,7 +50,11 @@ public:
* @param pPacket Packet to route
* @param down Response source
* @param reply Reply information
* @return True for success, false for error
*
* @return True for success, false for error. If the function returns false, the session will be closed.
*
* @throws May throw mxb::Exception. If the exception is thrown, the exception message is logged and the
* session will be closed.
*/
virtual bool clientReply(GWBUF&& packet, const mxs::ReplyRoute& down, const mxs::Reply& reply) = 0;

Expand Down
23 changes: 21 additions & 2 deletions server/core/service.cc
Expand Up @@ -1671,15 +1671,30 @@ bool ServiceEndpoint::clientReply(GWBUF&& buffer, const mxs::ReplyRoute& down, c
mxb::LogScope scope(m_service->name());
mxb_assert(m_open);
mxb_assert(buffer);
return m_router_session->clientReply(std::move(buffer), down, reply);
try
{
return m_router_session->clientReply(std::move(buffer), down, reply);
}
catch (const mxb::Exception& e)
{
MXB_ERROR("%s", e.what());
return false;
}
}

bool ServiceEndpoint::handleError(mxs::ErrorType type, const std::string& error,
mxs::Endpoint* down, const mxs::Reply& reply)
{
mxb::LogScope scope(m_service->name());
mxb_assert(m_open);
return m_router_session->handleError(type, error, down, reply);
try
{
return m_router_session->handleError(type, error, down, reply);
}
catch (const mxb::Exception& e)
{
return parent()->handleError(type, e.what(), this, reply);
}
}

void ServiceEndpoint::endpointConnReleased(Endpoint* down)
Expand All @@ -1699,6 +1714,10 @@ void ServiceEndpoint::call_handle_error(std::string_view errmsg)

if (ref->is_open())
{
// The call to handleError will go into another ServiceEndpoint or the MXS_SESSION which means it
// will not throw. Only the call to m_router_session->handleError() may throw as only modules are
// allowed to throw exceptions.
//
// The failure to route a query must currently be treated as a permanent error. If it is
// treated as a transient one, it could result in an infinite loop.
m_up->handleError(mxs::ErrorType::PERMANENT, err, this, mxs::Reply {});
Expand Down
3 changes: 3 additions & 0 deletions server/core/session.cc
Expand Up @@ -698,6 +698,9 @@ void MXS_SESSION::delay_routing(mxs::Routable* down, GWBUF&& buffer, std::chrono
}
catch (const mxb::Exception& e)
{
// The call to routeQuery threw an exception, propagate the error to the parent component.
// Since the parent component is always a mxs::Component, this call to handleError will now
// throw.
ref->parent()->handleError(mxs::ErrorType::PERMANENT, e.what(),
const_cast<mxs::Endpoint*>(ref.get()), mxs::Reply {});
}
Expand Down

0 comments on commit cfce684

Please sign in to comment.