Merge pull request praxis-live#57 from codelerity/perror-map

Update PError to use a lazily created PMap for text and serialization

praxiscore-api/src/main/java/org/praxislive/core/types/PError.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  * 
- * Copyright 2023 Neil C Smith.
+ * Copyright 2024 Neil C Smith.
  * 
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License version 3 only, as
@@ -23,11 +23,13 @@
 
 import java.util.Objects;
 import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.praxislive.core.Value;
 import org.praxislive.core.ValueFormatException;
 
 /**
- *
+ * An error message, possibly wrapping a Java Exception.
  */
 public final class PError extends Value {
 
@@ -36,47 +38,105 @@ public final class PError extends Value {
      */
     public static final String TYPE_NAME = "Error";
 
-    private final static String ERROR_PREFIX = "ERR:";
+    private static final String KEY_ERROR = "error";
+    private static final String KEY_MESSAGE = "message";
+    private static final String KEY_STACK_TRACE = "stack-trace";
 
-    private final Class<? extends Exception> type;
+    private final String type;
     private final String message;
     private final Exception ex;
 
-    private volatile String string;
+    private volatile String stack;
+    private volatile PMap data;
 
-    private PError(Class<? extends Exception> type,
-            String message, Exception ex, String str) {
+    private PError(String type, String message, Exception ex) {
+        this(type, message, ex, null, null);
+    }
+
+    private PError(String type, String message, Exception ex, String stack, PMap data) {
         this.type = type;
         this.message = message;
         this.ex = ex;
-        this.string = str;
+        this.stack = stack;
+        this.data = data;
     }
 
+    @Deprecated
     public Class<? extends Exception> exceptionType() {
+        return ex == null ? Exception.class : ex.getClass();
+    }
+
+    /**
+     * The error type, usually the simple name of a Java Exception.
+     *
+     * @return error type
+     */
+    public String errorType() {
         return type;
     }
 
+    /**
+     * The error message.
+     *
+     * @return error message
+     */
     public String message() {
         return message;
     }
 
+    /**
+     * A short form of the stack trace leading to this error, if available.
+     * Otherwise an empty String.
+     *
+     * @return stack trace or empty String.
+     */
+    public String stackTrace() {
+        if (stack == null) {
+            if (ex != null) {
+                stack = Stream.of(ex.getStackTrace())
+                        .skip(1)
+                        .limit(5)
+                        .map(e -> "    " + e.toString())
+                        .collect(Collectors.joining("\n"));
+            } else {
+                stack = "";
+            }
+        }
+        return stack;
+    }
+
+    /**
+     * The wrapped exception, if available. Direct access to the exception is
+     * only available in the process in which the error was created.
+     *
+     * @return optional exception
+     */
     public Optional<Exception> exception() {
         return Optional.ofNullable(ex);
     }
 
     @Override
     public String toString() {
-        String str = string;
-        if (str == null) {
-            StringBuilder sb = new StringBuilder(ERROR_PREFIX);
-            sb.append(' ')
-                    .append(type.getName())
-                    .append(' ')
-                    .append(Utils.escape(message));
-            str = sb.toString();
-            string = str;
+        return dataMap().toString();
+    }
+
+    /**
+     * The error as a {@link PMap}. This is similar to
+     * {@link PMap.MapBasedValue} except that the map representation of a PError
+     * is created lazily. A PError recreated from its PMap representation will
+     * not have an Exception reference.
+     *
+     * @return PError data as a PMap
+     */
+    public PMap dataMap() {
+        if (data == null) {
+            data = PMap.of(
+                    KEY_ERROR, errorType(),
+                    KEY_MESSAGE, message(),
+                    KEY_STACK_TRACE, stackTrace()
+            );
         }
-        return str;
+        return data;
     }
 
     @Override
@@ -100,81 +160,110 @@ public boolean equals(Object obj) {
         return false;
     }
 
-    public static PError parse(String str) throws ValueFormatException {
-        try {
-            PArray arr = PArray.parse(str);
-            if (arr.size() != 3 || !ERROR_PREFIX.equals(arr.get(0).toString())) {
-                throw new ValueFormatException();
-            }
-            ClassLoader cl = Thread.currentThread().getContextClassLoader();
-            Class<? extends Exception> type
-                    = (Class<? extends Exception>) cl.loadClass(arr.get(1).toString());
-            String msg = arr.get(2).toString();
-            return new PError(type, msg, null, str);
-        } catch (Exception ex) {
-            throw new ValueFormatException(ex);
-        }
-    }
-
-    static PError valueOf(String str) throws ValueFormatException {
-        try {
-            PArray arr = PArray.parse(str);
-            if (arr.size() != 3 || !ERROR_PREFIX.equals(arr.get(0).toString())) {
-                throw new ValueFormatException();
-            }
-            ClassLoader cl = Thread.currentThread().getContextClassLoader();
-            Class<? extends Exception> type
-                    = (Class<? extends Exception>) cl.loadClass(arr.get(1).toString());
-            String msg = arr.get(2).toString();
-            return new PError(type, msg, null, str);
-        } catch (Exception ex) {
-            throw new ValueFormatException(ex);
-        }
-    }
-
-    private static PError coerce(Value arg) throws ValueFormatException {
-        if (arg instanceof PError) {
-            return (PError) arg;
-        } else if (arg instanceof PReference) {
-            return PError.of(((PReference) arg).as(Exception.class)
-                    .orElseThrow(ValueFormatException::new));
-        }
-        return parse(arg.toString());
+    /**
+     * Parse a String as a PError. This first parses the text as a PMap.
+     *
+     * @param string text representation
+     * @return PError instance
+     * @throws ValueFormatException if the text cannot be parsed
+     */
+    public static PError parse(String string) throws ValueFormatException {
+        return fromMap(PMap.parse(string));
     }
 
-    public static Optional<PError> from(Value arg) {
+    /**
+     * Cast or convert the provided value into a PError, wrapped in an Optional.
+     * If the value is already a PError, the Optional will wrap the existing
+     * value. If the value is not a PError and cannot be converted into one, an
+     * empty Optional is returned.
+     *
+     * @param value value
+     * @return optional PError
+     */
+    public static Optional<PError> from(Value value) {
         try {
-            return Optional.of(coerce(arg));
+            return Optional.of(coerce(value));
         } catch (ValueFormatException ex) {
             return Optional.empty();
         }
     }
 
+    /**
+     * Create a PError wrapping the given Exception. The message will be taken
+     * from the exception, and the error type will be the simple name of the
+     * exception's class.
+     *
+     * @param ex exception
+     * @return PError instance
+     */
     public static PError of(Exception ex) {
-        Class<? extends Exception> type = ex.getClass();
+        String type = ex.getClass().getSimpleName();
         String msg = ex.getMessage();
         if (msg == null) {
             msg = "";
         }
-        return new PError(type, msg, ex, null);
+        return new PError(type, msg, ex);
     }
 
+    /**
+     * Create a PError wrapping the given Exception, with a custom message. The
+     * exception's message will be ignored. The error type will be the simple
+     * name of the exception's class.
+     *
+     * @param ex exception
+     * @param msg message
+     * @return PError instance
+     */
     public static PError of(Exception ex, String msg) {
-        return new PError(ex.getClass(),
+        return new PError(ex.getClass().getSimpleName(),
                 Objects.requireNonNull(msg),
-                ex,
-                null);
+                ex);
     }
 
+    /**
+     * Create a PError of the given message. The error type will be
+     * {@code Exception}.
+     *
+     * @param msg message
+     * @return PError instance
+     */
     public static PError of(String msg) {
         return of(Exception.class, msg);
     }
 
+    /**
+     * Create a PError of the given type and message. The error type will be the
+     * simple name of the passed in Exception type.
+     *
+     * @param type error type
+     * @param msg message
+     * @return PError instance
+     */
     public static PError of(Class<? extends Exception> type, String msg) {
-        return new PError(Objects.requireNonNull(type),
+        return new PError(type.getSimpleName(),
                 Objects.requireNonNull(msg),
-                null,
                 null);
     }
 
+    private static PError coerce(Value arg) throws ValueFormatException {
+        if (arg instanceof PError err) {
+            return err;
+        } else if (arg instanceof PReference ref) {
+            return PError.of(ref.as(Exception.class)
+                    .orElseThrow(ValueFormatException::new));
+        }
+        return fromMap(PMap.from(arg).orElseThrow(ValueFormatException::new));
+    }
+
+    private static PError fromMap(PMap data) throws ValueFormatException {
+        Value type = data.get(KEY_ERROR);
+        Value message = data.get(KEY_MESSAGE);
+        if (type == null || message == null) {
+            throw new ValueFormatException();
+        }
+        Value stack = data.get(KEY_STACK_TRACE);
+        return new PError(type.toString(), message.toString(), null,
+                stack == null ? "" : stack.toString(), data);
+    }
+
 }

praxiscore-api/src/main/java/org/praxislive/core/types/PMap.java

@@ -482,7 +482,7 @@ private static PMap coerce(Value arg) throws ValueFormatException {
      * empty Optional is returned.
      *
      * @param value value
-     * @return optional PArray
+     * @return optional PMap
      */
     public static Optional<PMap> from(Value value) {
         try {

praxiscore-hub-net/src/main/java/org/praxislive/hub/net/IonCodec.java

@@ -39,6 +39,7 @@
 import org.praxislive.core.types.PArray;
 import org.praxislive.core.types.PBoolean;
 import org.praxislive.core.types.PBytes;
+import org.praxislive.core.types.PError;
 import org.praxislive.core.types.PMap;
 import org.praxislive.core.types.PNumber;
 import org.praxislive.core.types.PString;
@@ -335,7 +336,8 @@ private Value readMapValue(String[] annotations, IonReader reader) throws Except
                     continue;
                 }
                 var vt = Value.Type.fromName(annotation).orElse(null);
-                if (vt != null && PMap.MapBasedValue.class.isAssignableFrom(vt.asClass())) {
+                if (vt != null && (PError.class == vt.asClass()
+                        || PMap.MapBasedValue.class.isAssignableFrom(vt.asClass()))) {
                     type = vt;
                     break;
                 }
@@ -385,6 +387,8 @@ private void writeValue(IonWriter writer, Value value) throws IOException {
             writer.writeBool(b.value());
         } else if (value instanceof PMap m) {
             writeMap(writer, m, TYPE_MAP);
+        } else if (value instanceof PError e) {
+            writeMap(writer, e.dataMap(), PError.TYPE_NAME, TYPE_MAP);
         } else if (value instanceof PMap.MapBasedValue v) {
             writeMap(writer, v.dataMap(), v.type().name(), TYPE_MAP);
         } else {
@@ -400,7 +404,7 @@ private void writeNumber(IonWriter writer, PNumber number) throws IOException {
         }
     }
 
-    private void writeMap(IonWriter writer, PMap map, String ... annotations) throws IOException {
+    private void writeMap(IonWriter writer, PMap map, String... annotations) throws IOException {
         writer.setTypeAnnotations(annotations);
         writer.stepIn(IonType.LIST);
         for (var key : map.keys()) {

praxiscore-hub-net/src/test/java/org/praxislive/hub/net/IonCodecTest.java

@@ -135,8 +135,10 @@ public void testErrorMessage() throws Exception {
         assertEquals(1, msgList.size());
         var decoded = (Message.Error) msgList.get(0);
         assertEquals(matchID, decoded.matchID());
-        var decodedError = PError.from(decoded.args().get(0)).orElseThrow();
-        assertEquals(IllegalStateException.class, decodedError.exceptionType());
+        var decodedArg = decoded.args().get(0);
+        assertTrue(decodedArg instanceof PError);
+        var decodedError = PError.from(decodedArg).orElseThrow();
+        assertEquals(IllegalStateException.class.getSimpleName(), decodedError.errorType());
         assertEquals("FOO", decodedError.message());
     }