[DISCUSS] Server Error Response Behaviour

[Cole Greer <Co...@improving.com.INVALID>]

Hi everyone,

A question came up on this PR<https://github.com/apache/tinkerpop/pull/1946> which I think is worthy of a broader discussion. The current state of the server is that if an uncaught error or an exception is thrown during a gremlin script traversal, the server will return that error/exception (stack trace and all) to the client in its response message. It will do the same thing if an exception is thrown during a bytecode traversal, but currently the server will never reply at all if an error is thrown during bytecode traversal. The PR in question fixes this by having bytecode traversals also return errors but it has opened the question of if this is truly the right behaviour.

I think there is a good argument that we shouldn’t be giving clients too much information about how certain requests may have been able to break things on the server. Using the example of a StackOverflow on the server, I think it would be sufficient if the server logged the StackOverflowError and then sent some sort of generic “TraversalExecutionFailedException” to the client. There are certainly many exceptions such as evaluation timeouts and malformed query exceptions which we would still want to surface to the client with some specificity. If we do think it’s best to obfuscate some errors, there will need to be some consideration of exactly where the line should be drawn between failures we should transparently surface and ones which should be replaced with a generic failure message.

I’m wondering if anyone else has any opinions on this. Does anyone find it valuable to surface every error in full detail to clients? Do people believe the server should not reveal that much detail? Does anyone have other suggestions as to how the server should behave when a traversal fails?

If there is no traction on this thread, I will assume a lazy consensus supporting the status quo and that no changes should be made other than adding error responses to bytecode traversals.

Regards,

Cole Greer

Re: [DISCUSS] Server Error Response Behaviour

[Cole Greer <Co...@improving.com.INVALID>]

Hi Divij,

Thanks for your response. I think you framed it really well. I agree that errors/exceptions should be evaluated through the lens of “is this useful for a user”. There is still an open question of exactly which errors and exceptions should be returned. My thoughts at this time is that by default, any Error gets replaced with “InternalServerError” unless the error is explicitly included in some list indicating it should be returned. I believe that Exceptions should work in the opposite manner where by default they are returned, unless explicitly excluded.

I believe that this is a separate issue from the previous problem of bytecode traversals not sending any response when Errors are thrown. As such I have created a new JIRA<https://issues.apache.org/jira/browse/TINKERPOP-2866> to track this and I don’t believe https://github.com/apache/tinkerpop/pull/1946 should be blocked by this.

I will update the new JIRA once I’ve had a chance to go through and consider individual errors and exceptions. Please let me know if you have any feedback or thoughts on the approach described above.

Thanks,

Cole Greer


From: Divij Vaidya <di...@gmail.com>
Date: Friday, February 3, 2023 at 12:34 AM
To: dev@tinkerpop.apache.org <de...@tinkerpop.apache.org>
Subject: Re: [DISCUSS] Server Error Response Behaviour
I would look at it from the lens of the user (gremlin client) experience.

The server should provide the details of the error if the user could either
take a corrective action or apply a workaround. As an example, in the
StackOverflowError error you quoted, a customer can reduce the length of
the query they are submitting to the server (which could be sometimes as
simple as replacing some steps with repeat()).

For errors which are not a function of user input or user configuration, we
can provide back a generic form of InternalServerError. An example here
would be when the underlying storage is unavailable. In such cases, a user
cannot take any action (unless they have access to the server in which case
we will call their role as operator), and hence, a generic retriable error
would suffice.

Thoughts?

--
Divij Vaidya



On Fri, Feb 3, 2023 at 3:01 AM Cole Greer <Co...@improving.com.invalid>
wrote:

> Hi everyone,
>
> A question came up on this PR<
> https://github.com/apache/tinkerpop/pull/1946><https://github.com/apache/tinkerpop/pull/1946%3e> which I think is worthy of
> a broader discussion. The current state of the server is that if an
> uncaught error or an exception is thrown during a gremlin script traversal,
> the server will return that error/exception (stack trace and all) to the
> client in its response message. It will do the same thing if an exception
> is thrown during a bytecode traversal, but currently the server will never
> reply at all if an error is thrown during bytecode traversal. The PR in
> question fixes this by having bytecode traversals also return errors but it
> has opened the question of if this is truly the right behaviour.
>
> I think there is a good argument that we shouldn’t be giving clients too
> much information about how certain requests may have been able to break
> things on the server. Using the example of a StackOverflow on the server, I
> think it would be sufficient if the server logged the StackOverflowError
> and then sent some sort of generic “TraversalExecutionFailedException” to
> the client. There are certainly many exceptions such as evaluation timeouts
> and malformed query exceptions which we would still want to surface to the
> client with some specificity. If we do think it’s best to obfuscate some
> errors, there will need to be some consideration of exactly where the line
> should be drawn between failures we should transparently surface and ones
> which should be replaced with a generic failure message.
>
> I’m wondering if anyone else has any opinions on this. Does anyone find it
> valuable to surface every error in full detail to clients? Do people
> believe the server should not reveal that much detail? Does anyone have
> other suggestions as to how the server should behave when a traversal fails?
>
> If there is no traction on this thread, I will assume a lazy consensus
> supporting the status quo and that no changes should be made other than
> adding error responses to bytecode traversals.
>
> Regards,
>
> Cole Greer
>
Warning: The sender of this message could not be validated and may not be the actual sender.

Re: [DISCUSS] Server Error Response Behaviour

[Divij Vaidya <di...@gmail.com>]

I would look at it from the lens of the user (gremlin client) experience.

The server should provide the details of the error if the user could either
take a corrective action or apply a workaround. As an example, in the
StackOverflowError error you quoted, a customer can reduce the length of
the query they are submitting to the server (which could be sometimes as
simple as replacing some steps with repeat()).

For errors which are not a function of user input or user configuration, we
can provide back a generic form of InternalServerError. An example here
would be when the underlying storage is unavailable. In such cases, a user
cannot take any action (unless they have access to the server in which case
we will call their role as operator), and hence, a generic retriable error
would suffice.

Thoughts?

--
Divij Vaidya



On Fri, Feb 3, 2023 at 3:01 AM Cole Greer <Co...@improving.com.invalid>
wrote:

> Hi everyone,
>
> A question came up on this PR<
> https://github.com/apache/tinkerpop/pull/1946> which I think is worthy of
> a broader discussion. The current state of the server is that if an
> uncaught error or an exception is thrown during a gremlin script traversal,
> the server will return that error/exception (stack trace and all) to the
> client in its response message. It will do the same thing if an exception
> is thrown during a bytecode traversal, but currently the server will never
> reply at all if an error is thrown during bytecode traversal. The PR in
> question fixes this by having bytecode traversals also return errors but it
> has opened the question of if this is truly the right behaviour.
>
> I think there is a good argument that we shouldn’t be giving clients too
> much information about how certain requests may have been able to break
> things on the server. Using the example of a StackOverflow on the server, I
> think it would be sufficient if the server logged the StackOverflowError
> and then sent some sort of generic “TraversalExecutionFailedException” to
> the client. There are certainly many exceptions such as evaluation timeouts
> and malformed query exceptions which we would still want to surface to the
> client with some specificity. If we do think it’s best to obfuscate some
> errors, there will need to be some consideration of exactly where the line
> should be drawn between failures we should transparently surface and ones
> which should be replaced with a generic failure message.
>
> I’m wondering if anyone else has any opinions on this. Does anyone find it
> valuable to surface every error in full detail to clients? Do people
> believe the server should not reveal that much detail? Does anyone have
> other suggestions as to how the server should behave when a traversal fails?
>
> If there is no traction on this thread, I will assume a lazy consensus
> supporting the status quo and that no changes should be made other than
> adding error responses to bytecode traversals.
>
> Regards,
>
> Cole Greer
>