From 8038ea523045a79c6e1e4c3300a6f28ddcc0d155 Mon Sep 17 00:00:00 2001
From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com>
Date: Sat, 22 Aug 2026 15:20:33 +0700
Subject: [PATCH] Assert what a failure carries rather than inspect it

The error model promises three things on every condition: the
GQLSTATUS code, the place in the statement, and the page that
documents it. Two of the three were untested here, and the third was
tested in a way that could not fail.

The position test read `e.position().ifPresent(p -> assertTrue(...))`,
which passes when there is no position at all. So did the caret. That
is the shape of assertion that turns a suite into a page of promises
with a green tick on it: the one outcome worth failing for is the one
it lets through. Where a field is genuinely optional now, the case
that has it and the case that does not are two tests and each says
which it is.

The doc URL was never checked anywhere. It is the field a program
hands a reader instead of five characters to go and search for, and
it is now asserted to be the base plus the code exactly as the code
is written. Exactly, because a condition class can have a letter in
it and a URL that lower cased it would be a 404 nobody would notice
until somebody clicked it.

Added with it: that the offset is where the token starts and a caller
who still has the text can slice at it, that the caret sits under the
column, that a name nothing bound is 42002 and not merely something
in class 42, that a connection used after close carries the misuse
status and deliberately carries no code and no page because it never
reached the engine, that a database that is not there names the path
it looked for, that the condition classes are subclasses of the one,
and that a diagnostic for a call that never happened leaves every
field empty rather than filling it with a zero that reads like a
place in the text.

Thirteen cases, run twice, once on each provider. Green on both
against libzu at the engine's HEAD.
---
 .../src/main/java/dev/zudb/tck/ErrorTest.java | 143 +++++++++++++++---
 1 file changed, 125 insertions(+), 18 deletions(-)

diff --git a/zudb-tck/src/main/java/dev/zudb/tck/ErrorTest.java b/zudb-tck/src/main/java/dev/zudb/tck/ErrorTest.java
index 6d36ec3..83d5d34 100644
--- a/zudb-tck/src/main/java/dev/zudb/tck/ErrorTest.java
+++ b/zudb-tck/src/main/java/dev/zudb/tck/ErrorTest.java
@@ -6,26 +6,49 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import dev.zudb.Config;
 import dev.zudb.Connection;
 import dev.zudb.Database;
+import dev.zudb.Diagnostic;
 import dev.zudb.Severity;
+import dev.zudb.Status;
+import dev.zudb.ZuClosedException;
+import dev.zudb.ZuConnectionException;
 import dev.zudb.ZuException;
+import dev.zudb.ZuProgrammingException;
 import dev.zudb.ZuSyntaxException;
+import java.nio.file.Path;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 /**
  * What a failure carries across the boundary.
  *
  * <p>The whole point of the error model is that a caller reads fields rather
  * than a message. These tests are what says the fields actually arrive.
+ *
+ * <p>They assert rather than inspect. A test written as {@code
+ * e.position().ifPresent(p -> assertTrue(...))} passes when the position is
+ * missing, which is the one outcome worth failing for, and a suite full of
+ * those is a page of promises with a green tick on it. Where a field is
+ * genuinely optional the case that has it and the case that does not are two
+ * tests, and each one says which it is.
  */
 public class ErrorTest {
 
+  /** Where a condition is written up. The engine puts the code on the end. */
+  private static final String DOCS = "https://zu.dev/docs/errors/";
+
+  /** A statement that fails to parse, with the mistake in the middle of it. */
+  private static final String MISSPELLED = "MATCH (p:person) RETRUN p";
+
   private static Database db;
   private static Connection conn;
 
+  @TempDir static Path dir;
+
   @BeforeAll
   static void engine() {
     Libzu.require();
@@ -45,9 +68,9 @@ static void done() {
 
   @Test
   void textThatWillNotParseIsASyntaxError() {
-    ZuSyntaxException e =
-        assertThrows(ZuSyntaxException.class, () -> conn.query("RETURN RETURN"));
+    ZuSyntaxException e = assertThrows(ZuSyntaxException.class, () -> conn.query(MISSPELLED));
     assertTrue(e.code().orElseThrow().startsWith("42"), e.code().orElseThrow());
+    assertFalse(e.condition().orElseThrow().isBlank());
     assertEquals(Severity.EXCEPTION, e.severity());
     assertFalse(e.retryable());
     assertNotNull(e.getMessage());
@@ -55,18 +78,71 @@ void textThatWillNotParseIsASyntaxError() {
   }
 
   @Test
-  void aFailureThatHasAPlaceCarriesIt() {
-    ZuException e = assertThrows(ZuException.class, () -> conn.query("RETURN RETURN"));
-    e.position()
-        .ifPresent(
-            p -> {
-              assertTrue(p.line() >= 1, "line " + p.line());
-              assertTrue(p.column() >= 1, "column " + p.column());
-              assertTrue(p.offset() >= 0, "offset " + p.offset());
-            });
-    // An excerpt and a column together are a caret, and the caret is the one
-    // piece of formatting this client does.
-    e.caret().ifPresent(c -> assertTrue(c.contains("^"), c));
+  void aFailureInTheTextCarriesThePlaceItIsIn() {
+    ZuException e = assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
+    ZuException.Position p = e.position().orElseThrow();
+    assertEquals(1, p.line());
+    assertTrue(p.column() > 1, "column " + p.column());
+    assertTrue(p.offset() > 0, "offset " + p.offset());
+    // The offset is where the token starts, said in bytes, and a caller who
+    // still has the text slices at it rather than counting columns.
+    assertTrue(MISSPELLED.substring(p.offset()).startsWith("RETRUN"), "offset " + p.offset());
+  }
+
+  @Test
+  void theDocUrlIsThePageForTheCode() {
+    ZuException e = assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
+    // Not a search box and not five characters to go and look up. The code
+    // goes on the end exactly as it is, because a condition class is written
+    // with a letter in it and a URL that lower cased it would be a 404.
+    assertEquals(DOCS + e.code().orElseThrow(), e.docUrl().orElseThrow());
+  }
+
+  @Test
+  void anExcerptAndACaretPointAtTheToken() {
+    ZuException e = assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
+    assertEquals(MISSPELLED, e.excerpt().orElseThrow());
+    String[] lines = e.caret().orElseThrow().split("\\R");
+    assertEquals(2, lines.length);
+    assertEquals(MISSPELLED, lines[0]);
+    assertEquals(e.position().orElseThrow().column() - 1, lines[1].indexOf('^'));
+  }
+
+  @Test
+  void aNameNothingBoundIsAReferenceError() {
+    ZuException e = assertThrows(ZuException.class, () -> conn.query("RETURN nobody AS x"));
+    assertEquals("42002", e.code().orElseThrow());
+    assertEquals(DOCS + "42002", e.docUrl().orElseThrow());
+  }
+
+  @Test
+  void aMistakeTheProgramMadeIsAProgrammingError() {
+    Database other = Database.memory();
+    Connection closed = other.connect();
+    closed.close();
+    ZuClosedException e =
+        assertThrows(ZuClosedException.class, () -> closed.query("RETURN 1 AS one"));
+    // A call that never reached the engine has no condition to name and says
+    // so, rather than borrowing one that would send a reader to the wrong
+    // page. The class is the diagnosis here, and the status behind it.
+    assertEquals(Status.MISUSE_CLOSED, e.status());
+    assertTrue(e.position().isEmpty());
+    assertTrue(e.docUrl().isEmpty());
+    assertFalse(e.getMessage().isBlank());
+    other.close();
+  }
+
+  @Test
+  void aDatabaseThatIsNotThereIsAConnectionError() {
+    Path missing = dir.resolve("not-here.zu1");
+    ZuConnectionException e =
+        assertThrows(
+            ZuConnectionException.class,
+            () -> Database.open(missing, Config.defaults().withReadOnly(true)));
+    assertFalse(e.getMessage().isBlank());
+    // The path is the one thing the caller cannot work out from the class,
+    // so a message that leaves it out is a message that starts a bisect.
+    assertTrue(e.getMessage().contains("not-here"), e.getMessage());
   }
 
   @Test
@@ -75,27 +151,58 @@ void everyFailureIsCatchableAsOneType() {
     assertThrows(ZuException.class, () -> conn.prepare("MATCH ("));
   }
 
+  @Test
+  void aConditionClassIsCatchableWithoutListingItsConditions() {
+    // The two characters that open a code are the condition class, and there
+    // is one subclass per class so that a condition added to class 42 later
+    // is caught by a catch somebody wrote today.
+    assertTrue(ZuException.class.isAssignableFrom(ZuSyntaxException.class));
+    assertTrue(ZuException.class.isAssignableFrom(ZuConnectionException.class));
+    assertTrue(ZuException.class.isAssignableFrom(ZuProgrammingException.class));
+    assertTrue(ZuProgrammingException.class.isAssignableFrom(ZuClosedException.class));
+    // Unchecked, which is the decision the class comment argues for.
+    assertTrue(RuntimeException.class.isAssignableFrom(ZuException.class));
+  }
+
   @Test
   void theDiagnosticIsTheWholeRecord() {
-    ZuException e = assertThrows(ZuException.class, () -> conn.query("RETURN RETURN"));
+    ZuException e = assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
     assertNotNull(e.diagnostic());
     assertEquals(e.getMessage(), e.diagnostic().message());
     assertEquals(e.status(), e.diagnostic().status());
+    assertEquals(e.code().orElseThrow(), e.diagnostic().code());
+    assertEquals(e.docUrl().orElseThrow(), e.diagnostic().docUrl());
+  }
+
+  @Test
+  void aConditionThatNeverHappenedLeavesTheFieldsEmpty() {
+    // The record a call that never reached the engine is built from. Nothing
+    // is guessed into it, because a position of zero would be a place in the
+    // text and there is no text.
+    ZuException e = Diagnostic.misuse(Status.MISUSE, "nothing in particular").toException();
+    assertEquals("nothing in particular", e.getMessage());
+    assertTrue(e.code().isEmpty());
+    assertTrue(e.condition().isEmpty());
+    assertTrue(e.position().isEmpty());
+    assertTrue(e.excerpt().isEmpty());
+    assertTrue(e.caret().isEmpty());
+    assertTrue(e.docUrl().isEmpty());
+    assertFalse(e.retryable());
   }
 
   @Test
   void aFailureLeavesTheConnectionUsable() {
-    assertThrows(ZuException.class, () -> conn.query("RETURN RETURN"));
+    assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
     conn.execute("RETURN 1 AS one");
     assertFalse(conn.isClosed());
   }
 
   @Test
-  void athousandFailuresLeakNothing() {
+  void aThousandFailuresLeakNothing() {
     // Every one of these allocates a zu_error on the far side. If the
     // binding forgot to free them this is where it would show.
     for (int i = 0; i < 1000; i++) {
-      assertThrows(ZuException.class, () -> conn.query("RETURN RETURN"));
+      assertThrows(ZuException.class, () -> conn.query(MISSPELLED));
     }
     conn.execute("RETURN 1 AS one");
   }