review(#10470): a refused catalog query keeps its text and logs no SEVERE in production mode, harden the IT teardown, pin the non-veto edge failure, reference #10473 at the relhasindex flag

Author: lvca

grpc-client/src/test/java/com/arcadedb/remote/grpc/Issue10373RemoteNewEdgeVetoIT.java

@@ -34,6 +34,7 @@
 import org.junit.jupiter.params.provider.EnumSource;
 
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -72,15 +73,20 @@ public void setTestConfiguration() {
   @Override
   public void endTest() {
     // HERE AND NOT IN A SEPARATE @AfterEach: THE ORDER OF TWO OF THEM IN ONE CLASS IS UNDEFINED, AND super.endTest() DROPS THE DATABASE
-    getServerDatabase(0, getDatabaseName()).getSchema().getType(EDGE).getEvents().unregisterListener(createVeto);
-    if (http != null)
-      http.close();
-    if (grpc != null)
-      grpc.close();
-    if (grpcServer != null)
-      grpcServer.close();
-    GlobalConfiguration.SERVER_PLUGINS.setValue("");
-    super.endTest();
+    // A SETUP THAT FAILED BEFORE THE EDGE TYPE OR THE LISTENER EXISTED MUST NOT KEEP THE SERVER UP AND THE PLUGIN SETTING SET
+    try {
+      if (createVeto != null && getServerDatabase(0, getDatabaseName()).getSchema().existsType(EDGE))
+        getServerDatabase(0, getDatabaseName()).getSchema().getType(EDGE).getEvents().unregisterListener(createVeto);
+      if (http != null)
+        http.close();
+      if (grpc != null)
+        grpc.close();
+      if (grpcServer != null)
+        grpcServer.close();
+    } finally {
+      GlobalConfiguration.SERVER_PLUGINS.setValue("");
+      super.endTest();
+    }
   }
 
   @BeforeEach
@@ -113,6 +119,22 @@ void aVetoedNewEdgeIsRaisedAsAVetoNamingTheEdgeAndBothEndpoints(final Driver dri
     assertThat(db.lookupByRID(fromRid, true).asVertex().countEdges(Vertex.DIRECTION.OUT, EDGE)).isZero();
   }
 
+  /**
+   * An empty CREATE EDGE answer is read as a veto because every other refusal reaches the client as a server error: pinned here, so
+   * a statement that fails for another reason (an edge type that does not exist) is never reported as a listener's veto.
+   */
+  @ParameterizedTest
+  @EnumSource(Driver.class)
+  void aRefusalThatIsNotAVetoIsNotReadAsOne(final Driver driver) {
+    final RemoteDatabase db = db(driver);
+    final RID fromRid = newVertex(db);
+    final RID toRid = newVertex(db);
+    final Vertex from = db.lookupByRID(fromRid, true).asVertex();
+
+    assertThatThrownBy(() -> from.newEdge("NoSuchEdgeType10373", toRid)).isNotInstanceOf(RecordVetoedException.class)
+        .isNotInstanceOf(NoSuchElementException.class);
+  }
+
   /** The remote message is the one the embedded engine builds, so a caller reads the same refusal on every path. */
   @ParameterizedTest
   @EnumSource(Driver.class)

postgresw/src/main/java/com/arcadedb/postgres/PostgresCatalog.java

@@ -150,11 +150,25 @@ public class PostgresCatalog {
    * that backtracks, sub-selects correlated deep enough to multiply. The client is told so, with PostgreSQL's
    * {@code program_limit_exceeded}, rather than handed an empty answer that would look like an empty catalog.
    */
-  public static final class WorkLimitExceeded extends RuntimeException {
+  public static final class WorkLimitExceeded extends Refusal {
     public static final String SQLSTATE = "54000";
 
     WorkLimitExceeded() {
-      super("The catalog query needs more work than one statement may spend: simplify it");
+      super("The catalog query needs more work than one statement may spend: simplify it", SQLSTATE);
+    }
+  }
+
+  /**
+   * A catalog query this server refuses, with the SQLSTATE the client acts on. The text is written by this class and holds no
+   * stored value (at most the name of a relation the client typed), and the cause is the client's query, never a fault of the
+   * server: so production mode keeps the message, as it does for a refused SET, and logs no SEVERE stack trace for it.
+   */
+  public abstract static class Refusal extends RuntimeException {
+    public final String sqlState;
+
+    Refusal(final String message, final String sqlState) {
+      super(message);
+      this.sqlState = sqlState;
     }
   }
 
@@ -171,11 +185,11 @@ public static final class WorkLimitExceeded extends RuntimeException {
    * with {@link WorkLimitExceeded} does not depend on why the statement was refused. PostgreSQL's own {@code undefined_table}
    * would claim the relation does not exist, which a real PostgreSQL has.
    */
-  public static final class UnsupportedCatalogQuery extends RuntimeException {
+  public static final class UnsupportedCatalogQuery extends Refusal {
     public static final String SQLSTATE = "0A000";
 
     UnsupportedCatalogQuery(final String message) {
-      super(message);
+      super(message, SQLSTATE);
     }
   }
 
@@ -1222,6 +1236,7 @@ private static Row tableRow(final Context context, final DocumentType type) {
     // relhasindex and hasindexes are FALSE because pg_index is not modelled (issue #10371): every query that would enumerate a
     // table's indexes names it and is refused, so a catalog that said "this relation has indexes" would send psql's \d, which
     // reads the flag first, straight into that refusal. The flag follows what this catalog can show, not what the type has.
+    // TODO #10473: model pg_index, then answer !type.getAllIndexes(false).isEmpty() again (here and in pg_tables.hasindexes)
     return singletonRow(context)//
         .with("pg_class", "oid", oid, "relname", type.getName(), "relnamespace", SCHEMA_OID, "relowner", OWNER_OID,//
             "relkind", "r", "relpersistence", "p", "relnatts", type.getPropertyNames().size(),//

postgresw/src/main/java/com/arcadedb/postgres/PostgresNetworkExecutor.java

@@ -4191,10 +4191,8 @@ static String sqlStateFor(final Throwable error) {
       return copy.sqlState;
     if (error instanceof PostgresSessionSettings.SettingException setting)
       return setting.sqlState;
-    if (error instanceof PostgresCatalog.WorkLimitExceeded)
-      return PostgresCatalog.WorkLimitExceeded.SQLSTATE; // program_limit_exceeded (issue #10228)
-    if (error instanceof PostgresCatalog.UnsupportedCatalogQuery)
-      return PostgresCatalog.UnsupportedCatalogQuery.SQLSTATE; // feature_not_supported (issue #10371)
+    if (error instanceof PostgresCatalog.Refusal refusal)
+      return refusal.sqlState; // program_limit_exceeded (issue #10228), feature_not_supported (issue #10371)
     return switch (ErrorCategory.of(error)) {
       // serialization_failure - the code drivers auto-retry on. Also a follower's refusal naming the leader (issue
       // #9845): the wire has no redirect code, so the leader is named in the message instead (see withLeaderRedirect)
@@ -4273,8 +4271,10 @@ static String withLeaderRedirect(final String text, final Throwable error) {
   /** {@link #clientMessage} without the leader redirect: the message, or the placeholder in production mode. */
   private String concealableMessage(final String message, final Throwable cause) {
     // A refused SET/DISCARD is text the protocol layer wrote, with no user data in it, and the explanation is what the
-    // client is owed; it is also a client mistake, never a server fault (issue #9327)
-    if (!server.isProductionMode() || cause instanceof PostgresSessionSettings.SettingException)
+    // client is owed; it is also a client mistake, never a server fault (issue #9327). A refused catalog query is the same: text
+    // this server wrote, with at most the relation the client named in it (issue #10371)
+    if (!server.isProductionMode() || cause instanceof PostgresSessionSettings.SettingException
+        || cause instanceof PostgresCatalog.Refusal)
       return message;
     // The client no longer sees the text, so the server log is the only copy the operator has. A routine, client-driven
     // failure (syntax error, duplicated key, ...) is not an incident: the message alone, no stack trace. Anything the

postgresw/src/test/java/com/arcadedb/postgres/Issue8931ProductionErrorConcealmentIT.java

@@ -120,6 +120,36 @@ void developmentModeKeepsTheFullMessage() throws Exception {
     assertThat(error.get('M')).contains(SECRET);
   }
 
+  /**
+   * A refused catalog query is text the protocol layer wrote, with no stored value in it (the relation named is the one the client
+   * typed), and a client mistake rather than a server fault: production mode keeps its explanation, as it does for a refused SET
+   * (issue #10371). Concealed, the client would read "an internal error happened" for a question it can only fix by asking another.
+   */
+  @Test
+  void aRefusedCatalogQueryKeepsItsExplanationInProductionMode() throws Exception {
+    final Map<Character, String> error = withMode("production", (out, in) -> {
+      sendSimpleQuery(out, "select count(*) from pg_catalog.pg_proc");
+      return failureOf(readUntilReadyForQuery(in));
+    });
+
+    assertThat(error.get('C')).isEqualTo("0A000");
+    assertThat(error.get('M')).contains("pg_proc").isNotEqualTo(ArcadeDBServer.CONCEALED_ERROR_MESSAGE);
+  }
+
+  /** The same for the refusal of a catalog query that costs too much (issue #10228): fixed text, a client-driven failure. */
+  @Test
+  void aCatalogQueryOverItsWorkBudgetKeepsItsExplanationInProductionMode() throws Exception {
+    final Map<Character, String> error = withMode("production", (out, in) -> {
+      sendSimpleQuery(out, "CREATE DOCUMENT TYPE " + "x".repeat(40) + " IF NOT EXISTS");
+      readUntilReadyForQuery(in);
+      sendSimpleQuery(out, "select relname from pg_catalog.pg_class where relname ~ '^(.*)(.*)(.*)(.*)(.*)Y$'");
+      return failureOf(readUntilReadyForQuery(in));
+    });
+
+    assertThat(error.get('C')).isEqualTo("54000");
+    assertThat(error.get('M')).contains("more work").isNotEqualTo(ArcadeDBServer.CONCEALED_ERROR_MESSAGE);
+  }
+
   private interface WireScript {
     Map<Character, String> run(DataOutputStream out, DataInputStream in) throws Exception;
   }