From ed8344d19ab303a46cb4e9fdb67340815c37e1c4 Mon Sep 17 00:00:00 2001 From: Nguyen Quang Minh <135627235+n24q02m@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:33:16 +0700 Subject: [PATCH 1/2] feat(api): return structured error code and hint for Cypher query failures Signed-off-by: Nguyen Quang Minh <135627235+n24q02m@users.noreply.github.com> --- .../hugegraph/api/cypher/CypherClient.java | 3 +- .../api/cypher/CypherErrorMapper.java | 67 +++++++++++++++++++ .../hugegraph/api/cypher/CypherModel.java | 27 +++++++- .../apache/hugegraph/api/CypherApiTest.java | 18 +++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java index 92ae18c54d..2a32790e02 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherClient.java @@ -80,7 +80,8 @@ public CypherModel submitQuery(String cypherQuery, @Nullable Map } catch (Exception e) { LOG.error(String.format("Failed to submit cypher-query: [ %s ], caused by:", cypherQuery), e); - res = CypherModel.failOf(request.getRequestId().toString(), e.getMessage()); + res = CypherModel.failOf(request.getRequestId().toString(), + e.getMessage(), CypherErrorMapper.map(e)); } finally { client.close(); cluster.close(); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java new file mode 100644 index 0000000000..f72889718d --- /dev/null +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java @@ -0,0 +1,67 @@ +/* + * 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; + +/** + * 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 MISSING_PARAMETER = + "HugeGraph.Cypher.MissingParameter"; + public static final String EXECUTION_ERROR = "HugeGraph.Cypher.ExecutionError"; + + 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(); + + if (lower.contains("not defined") && lower.contains("$")) { + return new CypherModel.CypherError(MISSING_PARAMETER, message, + "Provide the value via the 'parameters' query argument, " + + "e.g. ?parameters=%7B%22city%22%3A%20%22Beijing%22%7D"); + } + if (lower.contains("undefined vertex label") + || lower.contains("undefined edge label") + || lower.contains("undefined property key")) { + return new CypherModel.CypherError(EXECUTION_ERROR, message, + "Create the schema element via the schema API before " + + "running the query"); + } + if (lower.contains("not defined") || lower.contains("undefined")) { + return new CypherModel.CypherError(SYNTAX_ERROR, message, + "Declare the variable in a MATCH, UNWIND or WITH clause " + + "before referencing it"); + } + if (lower.contains("not supported") || lower.contains("unsupported")) { + 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, ""); + } +} diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java index e7c3900605..25f8f7a251 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java @@ -37,6 +37,9 @@ public class CypherModel { @Schema(description = "The query result") public Result result = new Result(); + @Schema(description = "Structured errors, empty on success") + public List errors = Collections.emptyList(); + public static CypherModel dataOf(String requestId, List data) { CypherModel res = new CypherModel(); res.requestId = requestId; @@ -45,11 +48,15 @@ public static CypherModel dataOf(String requestId, List data) { return res; } - public static CypherModel failOf(String requestId, String message) { + public static CypherModel failOf(String requestId, String message, + CypherError error) { CypherModel res = new CypherModel(); res.requestId = requestId; res.status.code = 400; res.status.message = message; + if (error != null) { + res.errors = Collections.singletonList(error); + } return res; } @@ -77,4 +84,22 @@ private static class Result { public Map meta = Collections.EMPTY_MAP; } + public static class CypherError { + + @Schema(description = "Stable error classification code") + public String code; + + @Schema(description = "The error message") + public String message; + + @Schema(description = "Actionable hint, empty when unavailable") + public String hint = ""; + + public CypherError(String code, String message, String hint) { + this.code = code; + this.message = message; + this.hint = hint; + } + } + } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java index 3c3e3049f3..e23d167f97 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java @@ -72,6 +72,24 @@ public void testRelationQuery() { this.testCypherQueryAndContains(cypher, "friend"); } + @Test + public void testSyntaxErrorHasStructuredErrorWithHint() { + Response r = client().post(PATH, + "MATCH (n:person) RETURN not_defined_var"); + String content = assertResponseStatus(200, r); + assertContains("\"errors\"", content); + assertContains("SyntaxError", content); + assertContains("Declare the variable", content); + } + + @Test + public void testUndefinedLabelHasSchemaHint() { + Response r = client().post(PATH, "MATCH (n:robot) RETURN n"); + String content = assertResponseStatus(200, r); + assertContains("ExecutionError", content); + assertContains("Create the schema element", content); + } + private void testCypherQueryAndContains(String cypher, String containsText) { Response r = client().post(PATH, cypher); this.validStatusAndTextContains(containsText, r); From a2ac2be83f4c27db38b06d28206a2b351e284d26 Mon Sep 17 00:00:00 2001 From: imbajin Date: Sat, 3 Oct 2026 01:17:13 +0800 Subject: [PATCH 2/2] fix(cypher): preserve legacy response compatibility (HG-19) --- docs/cypher-api.md | 119 ++++++++++++++ .../api/cypher/CypherErrorMapper.java | 29 ++-- .../hugegraph/api/cypher/CypherModel.java | 6 +- .../java/CypherClientCompatibilityTest.java | 93 +++++++++++ .../apache/hugegraph/api/CypherApiTest.java | 51 ++++-- .../apache/hugegraph/unit/UnitTestSuite.java | 4 + .../unit/api/cypher/CypherErrorTest.java | 150 ++++++++++++++++++ 7 files changed, 426 insertions(+), 26 deletions(-) create mode 100644 docs/cypher-api.md create mode 100644 hugegraph-server/hugegraph-test/src/compatibility/java/CypherClientCompatibilityTest.java create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/cypher/CypherErrorTest.java diff --git a/docs/cypher-api.md b/docs/cypher-api.md new file mode 100644 index 0000000000..0c5a9b9e15 --- /dev/null +++ b/docs/cypher-api.md @@ -0,0 +1,119 @@ +# Cypher API response compatibility + +The Cypher endpoint is +`/graphspaces/{graphspace}/graphs/{graph}/cypher`. GET accepts the `cypher` +query argument; POST accepts a raw Cypher statement with +`Content-Type: application/json`. Submit one statement per request. +The Apache mainline endpoint does not accept a parameter map. In the default +translator, unbound `$name` expressions become Cypher null; they do not raise a +missing-parameter error. Do not send `?parameters=...` to bind them. + +## 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 or unknown function | 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 reports `not supported` or `unsupported` | This construct is not covered by the built-in translator, see the Cypher compatibility guide for supported syntax | +| `HugeGraph.Cypher.ExecutionError` | Other failures | Empty string | + +The Gremlin transport exposes parser failures as messages, so classification +recognizes the translation-1.0.4 message forms above. Other failures fall back to +`ExecutionError`; these codes are not a promise to identify every possible +translator failure. 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. +The optional [bound-query fork #238](https://github.com/hugegraph/hugegraph/pull/238) +is independent and is not implemented here. + +## 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 -Dsurefire.failIfNoSpecifiedTests=false +``` + +`CypherApiTest` also exercises malformed syntax, multiple statements, undefined +variables and missing schema through a running test server. 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. diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java index f72889718d..7f321e994d 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherErrorMapper.java @@ -17,6 +17,9 @@ 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. @@ -26,10 +29,11 @@ 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 MISSING_PARAMETER = - "HugeGraph.Cypher.MissingParameter"; public static final String EXECUTION_ERROR = "HugeGraph.Cypher.ExecutionError"; + private static final Pattern UNDEFINED_VARIABLE = + Pattern.compile("variable `[^`]+` not defined"); + private CypherErrorMapper() { } @@ -38,25 +42,30 @@ public static CypherModel.CypherError map(Throwable e) { // 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(); + String lower = message.toLowerCase(Locale.ROOT); - if (lower.contains("not defined") && lower.contains("$")) { - return new CypherModel.CypherError(MISSING_PARAMETER, message, - "Provide the value via the 'parameters' query argument, " + - "e.g. ?parameters=%7B%22city%22%3A%20%22Beijing%22%7D"); - } if (lower.contains("undefined vertex label") || lower.contains("undefined edge label") - || lower.contains("undefined property key")) { + || 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 (lower.contains("not defined") || lower.contains("undefined")) { + 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")) { + 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.contains("not supported") || lower.contains("unsupported")) { return new CypherModel.CypherError(UNSUPPORTED_FEATURE, message, "This construct is not covered by the built-in translator, " + diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java index 25f8f7a251..f07995d8a4 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherModel.java @@ -37,9 +37,6 @@ public class CypherModel { @Schema(description = "The query result") public Result result = new Result(); - @Schema(description = "Structured errors, empty on success") - public List errors = Collections.emptyList(); - public static CypherModel dataOf(String requestId, List data) { CypherModel res = new CypherModel(); res.requestId = requestId; @@ -55,7 +52,8 @@ public static CypherModel failOf(String requestId, String message, res.status.code = 400; res.status.message = message; if (error != null) { - res.errors = Collections.singletonList(error); + res.status.attributes = Collections.singletonMap( + "errors", Collections.singletonList(error)); } return res; } diff --git a/hugegraph-server/hugegraph-test/src/compatibility/java/CypherClientCompatibilityTest.java b/hugegraph-server/hugegraph-test/src/compatibility/java/CypherClientCompatibilityTest.java new file mode 100644 index 0000000000..da51b8d639 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/compatibility/java/CypherClientCompatibilityTest.java @@ -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); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java index e23d167f97..4a0300db2e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java @@ -21,9 +21,12 @@ import java.util.Map; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import jakarta.ws.rs.core.Response; @@ -73,21 +76,45 @@ public void testRelationQuery() { } @Test - public void testSyntaxErrorHasStructuredErrorWithHint() { - Response r = client().post(PATH, - "MATCH (n:person) RETURN not_defined_var"); - String content = assertResponseStatus(200, r); - assertContains("\"errors\"", content); - assertContains("SyntaxError", content); - assertContains("Declare the variable", content); + public void testSyntaxErrorHasStructuredErrorWithHint() throws Exception { + this.assertError("MATCH (n:person) RETURN not_defined_var", + "HugeGraph.Cypher.SyntaxError", + "Declare the variable in a MATCH, UNWIND or WITH clause " + + "before referencing it"); } @Test - public void testUndefinedLabelHasSchemaHint() { - Response r = client().post(PATH, "MATCH (n:robot) RETURN n"); - String content = assertResponseStatus(200, r); - assertContains("ExecutionError", content); - assertContains("Create the schema element", content); + public void testParserErrorHasStructuredErrorWithHint() throws Exception { + this.assertError("MATCH (n:person RETURN n", "HugeGraph.Cypher.SyntaxError", + "Check the query syntax and function names supported by " + + "the built-in Cypher translator"); + } + + @Test + public void testMultipleStatementsHaveStructuredError() throws Exception { + this.assertError("MATCH (n) RETURN n; MATCH (m) RETURN m", + "HugeGraph.Cypher.SyntaxError", + "Submit exactly one Cypher statement per request"); + } + + @Test + public void testUndefinedLabelHasSchemaHint() throws Exception { + this.assertError("MATCH (n:robot) RETURN n", "HugeGraph.Cypher.ExecutionError", + "Create the schema element via the schema API before running the query"); + } + + private void assertError(String query, String code, String hint) throws Exception { + Response r = client().post(PATH, query); + JsonNode response = new ObjectMapper().readTree(assertResponseStatus(200, r)); + Assert.assertEquals(3, response.size()); + Assert.assertFalse(response.has("errors")); + Assert.assertEquals(400, response.at("/status/code").asInt()); + Assert.assertFalse(response.at("/status/message").asText().isEmpty()); + Assert.assertEquals(1, response.at("/status/attributes/errors").size()); + JsonNode error = response.at("/status/attributes/errors/0"); + Assert.assertEquals(code, error.get("code").asText()); + Assert.assertEquals(hint, error.get("hint").asText()); + Assert.assertFalse(error.get("message").asText().isEmpty()); } private void testCypherQueryAndContains(String cypher, String containsText) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 0e010ae5f7..f0f4ab08d8 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -31,6 +31,7 @@ import org.apache.hugegraph.store.client.OrderedScanSecurityTest; import org.apache.hugegraph.traversal.optimize.TraversalUtilOptimizeTest; import org.apache.hugegraph.unit.api.auth.LoginAPITest; +import org.apache.hugegraph.unit.api.cypher.CypherErrorTest; import org.apache.hugegraph.unit.api.filter.AccessLogFilterTest; import org.apache.hugegraph.unit.api.filter.LoadDetectFilterTest; import org.apache.hugegraph.unit.api.filter.PathFilterTest; @@ -115,6 +116,9 @@ LoginAPITest.class, PathFilterTest.class, + /* api cypher */ + CypherErrorTest.class, + /* api gremlin */ GremlinQueryAPITest.class, WsAndHttpBasicAuthHandlerTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/cypher/CypherErrorTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/cypher/CypherErrorTest.java new file mode 100644 index 0000000000..565f397d2e --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/cypher/CypherErrorTest.java @@ -0,0 +1,150 @@ +/* + * 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.unit.api.cypher; + +import java.util.Collections; +import java.util.Locale; +import java.util.concurrent.ExecutionException; + +import org.apache.hugegraph.api.cypher.CypherErrorMapper; +import org.apache.hugegraph.api.cypher.CypherModel; +import org.apache.tinkerpop.gremlin.driver.exception.ResponseException; +import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode; +import org.junit.Assert; +import org.junit.Test; +import org.opencypher.gremlin.translation.CypherAst; +import org.opencypher.gremlin.translation.translator.Translator; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +public class CypherErrorTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + + @Test + public void testSuccessEnvelope() throws Exception { + JsonNode response = JSON.readTree(JSON.writeValueAsString( + CypherModel.dataOf("request", Collections.singletonList("ok")))); + Assert.assertEquals(3, response.size()); + Assert.assertFalse(response.has("errors")); + Assert.assertEquals("request", response.get("requestId").asText()); + Assert.assertEquals(200, response.at("/status/code").asInt()); + Assert.assertEquals(0, response.at("/status/attributes").size()); + Assert.assertEquals("ok", response.at("/result/data/0").asText()); + } + + @Test + public void testFailureEnvelope() throws Exception { + CypherModel.CypherError error = CypherErrorMapper.map( + new IllegalArgumentException("Invalid input 'R'")); + JsonNode response = JSON.readTree(JSON.writeValueAsString( + CypherModel.failOf("request", "original message", error))); + Assert.assertEquals(3, response.size()); + Assert.assertFalse(response.has("errors")); + Assert.assertEquals(400, response.at("/status/code").asInt()); + Assert.assertEquals("original message", response.at("/status/message").asText()); + Assert.assertTrue(response.at("/result/data").isNull()); + JsonNode metadata = response.at("/status/attributes/errors/0"); + Assert.assertEquals("HugeGraph.Cypher.SyntaxError", metadata.get("code").asText()); + Assert.assertEquals(error.message, metadata.get("message").asText()); + Assert.assertEquals(error.hint, metadata.get("hint").asText()); + Assert.assertFalse(error.hint.isEmpty()); + } + + @Test + public void testFailureWithoutMetadata() throws Exception { + JsonNode response = JSON.readTree(JSON.writeValueAsString( + CypherModel.failOf("request", "failure", null))); + Assert.assertEquals(3, response.size()); + Assert.assertEquals(0, response.at("/status/attributes").size()); + Assert.assertEquals(400, response.at("/status/code").asInt()); + } + + @Test + public void testRealParserFailures() { + String[] queries = { + "MATCH (n:person RETURN n", + "MATCH (n:person) RETRUN n", + "MATCH (n) RETURN n; MATCH (m) RETURN m", + "RETURN date()", + "MATCH (n:person) RETURN not_defined_var" + }; + for (String query : queries) { + Exception failure = Assert.assertThrows(Exception.class, () -> translate(query)); + // Gremlin's response exception loses the parser's exception type. + CypherModel.CypherError error = CypherErrorMapper.map( + new ExecutionException(new ResponseException( + ResponseStatusCode.SERVER_ERROR, failure.getMessage()))); + Assert.assertEquals(query, "HugeGraph.Cypher.SyntaxError", error.code); + Assert.assertEquals(query, failure.getMessage(), error.message); + Assert.assertFalse(query, error.hint.isEmpty()); + Assert.assertEquals(query, query.contains("not_defined_var"), + error.hint.startsWith("Declare the variable")); + } + } + + @Test + public void testSchemaFailuresAreNotVariableErrors() { + for (String type : new String[]{"vertex label", "edge label", "property key", + "index label"}) { + CypherModel.CypherError error = CypherErrorMapper.map( + new IllegalArgumentException("Undefined " + type + ": 'x'")); + Assert.assertEquals("HugeGraph.Cypher.ExecutionError", error.code); + Assert.assertTrue(error.hint.startsWith("Create the schema element")); + } + Assert.assertEquals("HugeGraph.Cypher.ExecutionError", CypherErrorMapper.map( + new IllegalArgumentException("Resource not defined")).code); + } + + @Test + public void testUnsupportedAndFallback() { + Assert.assertEquals("HugeGraph.Cypher.UnsupportedFeature", CypherErrorMapper.map( + new UnsupportedOperationException("Feature not supported")).code); + Assert.assertEquals("HugeGraph.Cypher.UnsupportedFeature", CypherErrorMapper.map( + new UnsupportedOperationException("Unsupported construct")).code); + CypherModel.CypherError fallback = CypherErrorMapper.map(new RuntimeException()); + Assert.assertEquals("HugeGraph.Cypher.ExecutionError", fallback.code); + Assert.assertEquals("", fallback.hint); + Assert.assertFalse(fallback.message.isEmpty()); + } + + @Test + public void testParametersRemainUnbound() { + String gremlin = translate("MATCH (n:person) WHERE n.name = $name RETURN n"); + Assert.assertTrue(gremlin, gremlin.contains("cypher.null")); + } + + @Test + public void testClassificationIsLocaleIndependent() { + Locale previous = Locale.getDefault(); + try { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + Assert.assertEquals("HugeGraph.Cypher.SyntaxError", CypherErrorMapper.map( + new IllegalArgumentException("INVALID INPUT 'R'")).code); + } finally { + Locale.setDefault(previous); + } + } + + private static String translate(String query) { + return CypherAst.parse(query, Collections.emptyMap()).buildTranslation( + Translator.builder().gremlinGroovy() + .build("gremlin+cfog_server_extensions+inline_parameters")); + } +}