Repository navigation
fix(cypher): preserve legacy clients and classify parser errors (HG-19) #3259
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
404792f
c66f5e7
e3cd2e2
8a1422d
ebcd88e
e360d5e
3d44839
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| # Cypher API response compatibility | ||
|
|
||
| The Cypher endpoint is | ||
| `/graphspaces/{graphspace}/graphs/{graph}/cypher`. Submit one statement per | ||
| request. GET accepts the `cypher` query argument and does not accept a parameter | ||
| map; use POST for parameterized queries. POST with `Content-Type: application/json` | ||
| accepts a JSON object containing a nonblank string `cypher` and an optional | ||
| object `parameters`, for example: | ||
|
|
||
| ```json | ||
| { | ||
| "cypher": "MATCH (n:person) WHERE n.name = $name RETURN n.name", | ||
| "parameters": {"name": "marko"} | ||
| } | ||
| ``` | ||
|
|
||
| Legacy raw Cypher bodies sent as `application/json` remain accepted. JSON arrays, | ||
| scalars, malformed JSON, missing/non-string/blank `cypher`, and a `parameters` | ||
| value of null, an array or a scalar are rejected by HTTP request validation. | ||
| Omitting `parameters` supplies an empty map. Additional request fields and extra | ||
| parameter names are allowed; every parameter referenced by the parsed query must | ||
| be present. Individual parameter values may explicitly be null. | ||
| Requests are limited to 16 top-level parameter names. This limit is fixed; see | ||
| [Cypher HTTP contracts](server-runtime.md#cypher-http-contracts) for the binding | ||
| limits and validation rules. | ||
|
|
||
| The Server checks missing bindings before executing the traversal, so GET, raw | ||
| POST and JSON POST with an absent referenced name fail with `Missing Cypher | ||
| parameter: name`. This uses the execution-failure envelope below, classified as | ||
| `HugeGraph.Cypher.ExecutionError` with an empty hint. The upstream translator's | ||
| standalone unbound-to-null behavior is not the Server request contract. Do not | ||
| send `?parameters=...` to bind a GET query. Invalid binding values, including the | ||
| reserved translator null marker, also fail before traversal execution; explicit | ||
| JSON null values are accepted and normalized by the Server. | ||
|
|
||
| ## Response envelope | ||
|
|
||
| Responses retain the Gremlin-compatible top-level fields `requestId`, `status` | ||
| and `result`. There is no top-level `errors` field. Existing Java clients can | ||
| continue to deserialize both successful and failed queries without an upgrade. | ||
|
|
||
| A successful query has `status.code = 200`, an empty `status.attributes` map, | ||
| and its rows in `result.data`, for example: | ||
|
|
||
| ```json | ||
| { | ||
| "requestId": "example-request", | ||
| "status": {"message": "", "code": 200, "attributes": {}}, | ||
| "result": {"data": [{"value": 1}], "meta": {}} | ||
| } | ||
| ``` | ||
|
|
||
| When query submission or execution fails inside the Cypher client, the existing | ||
| HTTP 200 / `status.code = 400` convention and `status.message` are preserved. | ||
| Authentication, request validation and other HTTP-layer errors remain separate | ||
| and need not use this envelope. Consumers must inspect `status.code` instead of | ||
| relying only on the HTTP status. | ||
|
|
||
| Structured error metadata lives at `status.attributes.errors`, an array with | ||
| one entry for the failed query. Each entry contains a stable string `code`, the | ||
| translator/execution `message`, and a `hint` (empty when no specific guidance is | ||
| available). Message wording is not a stable contract. Example: | ||
|
|
||
| ```json | ||
| { | ||
| "requestId": "example-request", | ||
| "status": { | ||
| "message": "Expected exactly one statement per query but got: 2", | ||
| "code": 400, | ||
| "attributes": { | ||
| "errors": [{ | ||
| "code": "HugeGraph.Cypher.SyntaxError", | ||
| "message": "Expected exactly one statement per query but got: 2", | ||
| "hint": "Submit exactly one Cypher statement per request" | ||
| }] | ||
| } | ||
| }, | ||
| "result": {"data": null, "meta": {}} | ||
| } | ||
| ``` | ||
|
|
||
| Clients that only consume `status.code`, `status.message` and `result` can ignore | ||
| the attributes. A failure with no mapped metadata may have empty attributes. | ||
|
|
||
| ## Error codes and hints | ||
|
|
||
| | Code | Recognized failure | Hint | | ||
| | --- | --- | --- | | ||
| | `HugeGraph.Cypher.SyntaxError` | Parser invalid input, unknown function or semantic error with a `(line N, column M (offset: K))` suffix | Check the query syntax and function names supported by the built-in Cypher translator | | ||
| | `HugeGraph.Cypher.SyntaxError` | More or fewer than one statement | Submit exactly one Cypher statement per request | | ||
| | `HugeGraph.Cypher.SyntaxError` | `Variable` followed by a backtick-quoted name and `not defined` | Declare the variable in a MATCH, UNWIND or WITH clause before referencing it | | ||
| | `HugeGraph.Cypher.ExecutionError` | Undefined vertex label, edge label, property key or index label | Create the schema element via the schema API before running the query | | ||
| | `HugeGraph.Cypher.UnsupportedFeature` | Translator messages beginning `Unsupported clause:`, `Unsupported match pattern:`, `Unsupported create pattern:`, `Unsupported property type:`, `Multiple labels are not supported:` or `Custom functions and predicates are not supported on target implementation:` | This construct is not covered by the built-in translator, see the Cypher compatibility guide for supported syntax | | ||
| | `HugeGraph.Cypher.ExecutionError` | Other failures, including unrecognized translation failures | Empty string | | ||
|
|
||
| `ExecutionError` is also the fallback for unrecognized translation failures; it | ||
| does not mean traversal execution has started. For example, `reduce(...)` or | ||
| semantic rejections without a recognized position suffix may return this code | ||
| with an empty hint while preserving the original failure message. Classification | ||
| recognizes only the translation-1.0.4 forms above and does not cover every | ||
| translator failure; runtime errors such as `Unsupported predicate` and | ||
| `CONTAINS query ... is not supported` also fall back. | ||
| There is no `HugeGraph.Cypher.MissingParameter` contract. | ||
|
|
||
| This contract repairs the unreleased proposal in | ||
| [Server #3241](https://github.com/apache/hugegraph/pull/3241). The paired website | ||
| work is [Doc #499](https://github.com/apache/hugegraph-doc/pull/499); its author | ||
| must incorporate this response contract before coordinated publication. | ||
| Parameterized POST requests are already provided by merged | ||
| [Server #3289](https://github.com/apache/hugegraph/pull/3289). | ||
| The earlier [bound-query fork #238](https://github.com/hugegraph/hugegraph/pull/238) | ||
| is a historical reference; use the current request contract above. | ||
|
|
||
| ## Regression checks | ||
|
|
||
| Run the mapper, real-parser and envelope tests from the repository root: | ||
|
|
||
| ```sh | ||
| mvn test -pl hugegraph-server/hugegraph-test -am -P unit-test \ | ||
| -Dtest=CypherErrorTest,CypherClientTest,CypherOpProcessorTest \ | ||
| -Dsurefire.failIfNoSpecifiedTests=false | ||
| ``` | ||
|
|
||
| `CypherApiTest` also exercises malformed syntax, multiple statements, undefined | ||
| variables and missing schema through a running test server. Its `testParameters` | ||
| and `testMissingParameterHasFallbackError` cover bound JSON requests and missing | ||
| bindings. `CypherClientTest` checks outgoing query/binding transport, and | ||
| `CypherOpProcessorTest` covers request binding validation. The mapper's | ||
| translator-only unbound-parameter test bypasses that validation. Follow the normal | ||
| Server API-test setup using disposable test data. | ||
|
|
||
| The standalone `CypherClientCompatibilityTest` uses the actual Java Client | ||
| `Response` and `RestResult` classes. It is outside the Server reactor's source | ||
| roots to avoid a Server-to-Client dependency cycle. Supply an existing Client | ||
| jar (checked with locally available 1.7.0 and development 1.8.0 artifacts) and run from the repository root: | ||
|
|
||
| ```sh | ||
| mvn -f hugegraph-server/hugegraph-api/pom.xml dependency:build-classpath \ | ||
| -Dmdep.outputFile="$PWD/target/cypher-classpath.txt" | ||
| CYPHER_CLIENT_JAR="$HOME/.m2/repository/org/apache/hugegraph/hugegraph-client/1.7.0/hugegraph-client-1.7.0.jar" | ||
| CYPHER_CP="$CYPHER_CLIENT_JAR:$(cat target/cypher-classpath.txt)" | ||
| mkdir -p target/cypher-compatibility | ||
| javac -encoding UTF-8 -proc:none -cp "$CYPHER_CP" \ | ||
| -d target/cypher-compatibility \ | ||
| hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java \ | ||
| hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java \ | ||
| hugegraph-server/hugegraph-test/src/compatibility/java/CypherClientCompatibilityTest.java | ||
| java -cp "target/cypher-compatibility:$CYPHER_CP" \ | ||
| org.junit.runner.JUnitCore CypherClientCompatibilityTest | ||
| ``` | ||
|
|
||
| These checks cover legacy envelopes, current success/failure deserialization, | ||
| status/message preservation, and a negative control proving that a top-level | ||
| `errors` field is rejected. They do not certify a live Hubble deployment or | ||
| matching-release-candidate acceptance. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.hugegraph.api.cypher; | ||
|
|
||
| import java.util.Locale; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * Classify cypher execution failures into stable error codes and attach | ||
| * actionable hints for the most common, cryptic translator messages. | ||
| */ | ||
| public final class CypherErrorMapper { | ||
|
|
||
| public static final String SYNTAX_ERROR = "HugeGraph.Cypher.SyntaxError"; | ||
| public static final String UNSUPPORTED_FEATURE = | ||
| "HugeGraph.Cypher.UnsupportedFeature"; | ||
| public static final String EXECUTION_ERROR = "HugeGraph.Cypher.ExecutionError"; | ||
|
|
||
| private static final Pattern UNDEFINED_VARIABLE = | ||
| Pattern.compile("variable `[^`]+` not defined"); | ||
|
|
||
| private static final Pattern PARSER_POSITION = | ||
| Pattern.compile("\\(line \\d+, column \\d+ \\(offset: \\d+\\)\\)$"); | ||
|
|
||
| private CypherErrorMapper() { | ||
| } | ||
|
|
||
| public static CypherModel.CypherError map(Throwable e) { | ||
| String message = e.getMessage() != null ? e.getMessage() : e.toString(); | ||
| // strip noisy exception class prefixes, e.g. "...driver.exception.ResponseException: " | ||
| message = message.replaceFirst( | ||
| "^([a-zA-Z0-9_]+\\.)+[A-Za-z0-9_]+(Exception|Error):\\s*", ""); | ||
| String lower = message.toLowerCase(Locale.ROOT); | ||
|
|
||
| if (lower.contains("undefined vertex label") | ||
| || lower.contains("undefined edge label") | ||
| || lower.contains("undefined property key") | ||
| || lower.contains("undefined index label")) { | ||
| return new CypherModel.CypherError(EXECUTION_ERROR, message, | ||
| "Create the schema element via the schema API before " + | ||
| "running the query"); | ||
| } | ||
| if (UNDEFINED_VARIABLE.matcher(lower).find()) { | ||
| return new CypherModel.CypherError(SYNTAX_ERROR, message, | ||
| "Declare the variable in a MATCH, UNWIND or WITH clause " + | ||
| "before referencing it"); | ||
| } | ||
| if (lower.contains("invalid input") || lower.contains("unknown function") || | ||
| PARSER_POSITION.matcher(lower).find()) { | ||
| return new CypherModel.CypherError(SYNTAX_ERROR, message, | ||
| "Check the query syntax and function names supported by " + | ||
| "the built-in Cypher translator"); | ||
| } | ||
| if (lower.contains("expected exactly one statement")) { | ||
| return new CypherModel.CypherError(SYNTAX_ERROR, message, | ||
| "Submit exactly one Cypher statement per request"); | ||
| } | ||
| if (lower.startsWith("unsupported clause:") || | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important: The allow-list covers only a few of the translator's own unsupported forms, so common gaps now map to ExecutionError with an empty hint. translation-1.0.4 throws "Unsupported : " from WalkerContext.unsupported() for about twenty descriptions; the list matches clause, match pattern and create pattern. Probe at e360d5e through the evalCypher translation path: reduce(s = 0, x IN [1,2] | s + x) gives "Unsupported value expression", SET n:Foo:Bar and REMOVE n:a:b give "Unsupported set clause", DELETE n DELETE n gives "Unsupported query. Multiple delete clauses", gremlin('g.V()') gives "Unsupported Add the remaining descriptions (value expression, expression, set clause, projection, query,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed with a small probe through the actual CypherOpProcessor, native Gremlin response handlers and driver ResponseException. The remote exception types are preserved, but reduce/SET/DELETE translation gaps use the same generic UnsupportedOperationException and native response status as the backend negative control; the mapper cannot reliably infer the stage from that type alone. The documentation now states that ExecutionError can also be a translation fallback and does not prove execution began. This does not implement complete UnsupportedFeature classification. I am keeping this thread open while the scope of using the existing translation boundary is decided, rather than extending the message allow-list or misclassifying backend errors.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user has now explicitly accepted the documented limitation for this PR, with complete translation-stage classification deferred to a separate follow-up. The current documentation states that unrecognized translation failures, including reduce and semantic rejections without a recognized position suffix, can return ExecutionError with an empty hint; that code does not prove traversal execution began, and the original message is preserved. This is an accepted scope boundary, not an implementation fix. The mapper and processor remain unchanged, and the requested additional UnsupportedFeature coverage has not been implemented. The full-classification work is recorded separately, and I am leaving this Important thread open. |
||
| lower.startsWith("unsupported match pattern:") || | ||
| lower.startsWith("unsupported create pattern:") || | ||
| lower.startsWith("unsupported property type:") || | ||
| lower.startsWith("multiple labels are not supported:") || | ||
| lower.startsWith("custom functions and predicates are not supported on target implementation:")) { | ||
| return new CypherModel.CypherError(UNSUPPORTED_FEATURE, message, | ||
| "This construct is not covered by the built-in translator, " + | ||
| "see the Cypher compatibility guide for supported syntax"); | ||
| } | ||
| return new CypherModel.CypherError(EXECUTION_ERROR, message, ""); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| import java.util.Collections; | ||
|
|
||
| import org.apache.hugegraph.api.cypher.CypherErrorMapper; | ||
| import org.apache.hugegraph.api.cypher.CypherModel; | ||
| import org.apache.hugegraph.rest.RestResult; | ||
| import org.apache.hugegraph.structure.gremlin.Response; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.node.ObjectNode; | ||
|
|
||
| /** | ||
| * Run with an existing hugegraph-client jar (see docs/cypher-api.md). | ||
| * Kept outside the Server reactor to avoid a Server -> Client dependency cycle. | ||
| */ | ||
| public class CypherClientCompatibilityTest { | ||
|
|
||
| private static final ObjectMapper JSON = new ObjectMapper(); | ||
|
|
||
| @Test | ||
| public void testLegacySuccessAndFailure() { | ||
| for (int code : new int[]{200, 400}) { | ||
| String body = "{\"requestId\":\"request\",\"status\":{\"code\":" + code + | ||
| ",\"message\":\"original\",\"attributes\":{}}," + | ||
| "\"result\":{\"data\":" + (code == 200 ? "[]" : "null") + | ||
| ",\"meta\":{}}}"; | ||
| Response response = read(body); | ||
| Assert.assertEquals(code, response.status().code()); | ||
| Assert.assertEquals("original", response.status().message()); | ||
| Assert.assertTrue(response.status().attributes().isEmpty()); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testCurrentSuccess() throws Exception { | ||
| Response response = read(JSON.writeValueAsString(CypherModel.dataOf( | ||
| "request", Collections.singletonList(Collections.singletonMap("value", 1))))); | ||
| Assert.assertEquals(200, response.status().code()); | ||
| Assert.assertEquals("request", response.requestId()); | ||
| Assert.assertTrue(response.status().attributes().isEmpty()); | ||
| Assert.assertEquals(1, response.result().size()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCurrentFailure() throws Exception { | ||
| CypherModel.CypherError error = CypherErrorMapper.map( | ||
| new IllegalArgumentException("Invalid input 'R'")); | ||
| Response response = read(JSON.writeValueAsString( | ||
| CypherModel.failOf("request", "original failure", error))); | ||
| // CypherManager checks this status before handing the result to Hubble. | ||
| Assert.assertEquals(400, response.status().code()); | ||
| Assert.assertEquals("original failure", response.status().message()); | ||
| Assert.assertEquals("HugeGraph.Cypher.SyntaxError", JSON.valueToTree( | ||
| response.status().attributes()).at("/errors/0/code").asText()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testTopLevelErrorsNegativeControl() throws Exception { | ||
| ObjectNode body = (ObjectNode) JSON.readTree(JSON.writeValueAsString( | ||
| CypherModel.dataOf("request", Collections.emptyList()))); | ||
| body.putArray("errors"); | ||
| RuntimeException failure = Assert.assertThrows(RuntimeException.class, | ||
| () -> read(body.toString())); | ||
| Throwable cause = failure; | ||
| while (cause.getCause() != null) { | ||
| cause = cause.getCause(); | ||
| } | ||
| Assert.assertEquals("UnrecognizedPropertyException", cause.getClass().getSimpleName()); | ||
| Assert.assertTrue(cause.getMessage().contains("errors")); | ||
| } | ||
|
|
||
| private static Response read(String body) { | ||
| return new RestResult(200, body, null).readObject(Response.class); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.