You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2023/01/16 22:10:39 UTC

[GitHub] [tinkerpop] Cole-Greer opened a new pull request, #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Cole-Greer opened a new pull request, #1946:
URL: https://github.com/apache/tinkerpop/pull/1946

   https://issues.apache.org/jira/browse/TINKERPOP-2767
   
   The existing error handling in TraversalOpProcessor and SessionOpProcessor during bytecode iteration was catching all exceptions sending an appropriate error response to the client. The OpProcessor however was not catching a StackOverflowError which could be induced by running a query which contains a large repeat step. This Error was being caught by FutureTask.run() but GremlinServer never wait's on this task or checks the results which caused this Error to be lost. A similar issue was found to exist in AbstractSession.process().
   
   This PR adjusts the existing error handling code to catch any Throwable during bytecode iteration so clients will receive error messages and codes for server errors as well as exceptions. Any errors are re-thrown such that the evalFuture FutureTask will continue to have an exception set correctly (although GremlinServer currently does not use this for anything).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] xiazcy commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1421428035

   I don't have a problem with this change. I'd add a CHANGELOG entry since this is alters expected error output on driver. Otherwise VOTE +1. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] xiazcy merged pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy merged PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] Cole-Greer commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1414590829

   > 
   
   I structured this PR in this manner to mirror the behaviour of gremlin script executions in addition to this being the solution with the smallest changes. I think it's fair to have a discussion if this is actually the ideal behaviour. Perhaps this is worthy of a discussion in the dev list? I believe if we decide a different response behaviour is preferable for bytecode traversals, that we should apply that same change to script-based traversals. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] xiazcy commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1414555247

   I think there is the question of do we want to send all of the error messages to the client? I feel like we should handle the hanging in the driver, but it may not be necessary to dump all stack trace?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] vkagamlyk commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1424712778

   VOTE +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] xiazcy commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1439298787

   Just to note that we'll need a PR to 3.6-dev separately for these changes. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] Cole-Greer commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1421410945

   Based on discussion in the dev list, I created [TINKERPOP-2866](https://issues.apache.org/jira/browse/TINKERPOP-2866) which I would consider separate from the issue this PR is resolving. I would ask that we proceed with merging this PR if there are no objections.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] codecov-commenter commented on pull request #1946: [TINKERPOP-2767] Fix server not handling StackOverflowError in bytecode traversals

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1946:
URL: https://github.com/apache/tinkerpop/pull/1946#issuecomment-1384628526

   # [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1946](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1972653) into [3.5-dev](https://codecov.io/gh/apache/tinkerpop/commit/b8e6b2827d68bfbc73205df00f201a0e75bd7ca2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b8e6b28) will **decrease** coverage by `5.60%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             3.5-dev    #1946      +/-   ##
   =============================================
   - Coverage      69.29%   63.69%   -5.61%     
   =============================================
     Files            865       24     -841     
     Lines          41086     3680   -37406     
     Branches        5417        0    -5417     
   =============================================
   - Hits           28472     2344   -26128     
   + Misses         10696     1159    -9537     
   + Partials        1918      177    -1741     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...kerpop/gremlin/server/handler/AbstractSession.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9oYW5kbGVyL0Fic3RyYWN0U2Vzc2lvbi5qYXZh) | | |
   | [.../gremlin/server/op/session/SessionOpProcessor.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9vcC9zZXNzaW9uL1Nlc3Npb25PcFByb2Nlc3Nvci5qYXZh) | | |
   | [...mlin/server/op/traversal/TraversalOpProcessor.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9vcC90cmF2ZXJzYWwvVHJhdmVyc2FsT3BQcm9jZXNzb3IuamF2YQ==) | | |
   | [...pop/gremlin/process/traversal/TraversalSource.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9UcmF2ZXJzYWxTb3VyY2UuamF2YQ==) | | |
   | [...n/language/grammar/ParseTreeContextCastHelper.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9sYW5ndWFnZS9ncmFtbWFyL1BhcnNlVHJlZUNvbnRleHRDYXN0SGVscGVyLmphdmE=) | | |
   | [...remlin/structure/io/graphson/JsonParserConcat.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vZ3JhcGhzb24vSnNvblBhcnNlckNvbmNhdC5qYXZh) | | |
   | [...mlin/process/traversal/dsl/ProcessorException.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9kc2wvUHJvY2Vzc29yRXhjZXB0aW9uLmphdmE=) | | |
   | [...traversal/step/map/TraversalVertexProgramStep.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL2NvbXB1dGVyL3RyYXZlcnNhbC9zdGVwL21hcC9UcmF2ZXJzYWxWZXJ0ZXhQcm9ncmFtU3RlcC5qYXZh) | | |
   | [...lin/structure/io/binary/types/ClassSerializer.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vYmluYXJ5L3R5cGVzL0NsYXNzU2VyaWFsaXplci5qYXZh) | | |
   | [...e/tinkerpop/gremlin/server/util/LifeCycleHook.java](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci91dGlsL0xpZmVDeWNsZUhvb2suamF2YQ==) | | |
   | ... and [831 more](https://codecov.io/gh/apache/tinkerpop/pull/1946?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org