From 4ca178a888a5299e761f86e5e813ba24c6399a49 Mon Sep 17 00:00:00 2001 From: Neil C Smith Date: Thu, 16 May 2024 17:53:55 +0100 Subject: [PATCH] Update PError to use a lazily created PMap as text and serialization format. --- .../org/praxislive/core/types/PError.java | 223 ++++++++++++------ .../java/org/praxislive/core/types/PMap.java | 2 +- .../java/org/praxislive/hub/net/IonCodec.java | 8 +- .../org/praxislive/hub/net/IonCodecTest.java | 6 +- .../praxislive/launcher/LogServiceImpl.java | 39 ++- .../praxislive/script/ScriptStackFrame.java | 2 +- .../tests/core/async-functions/data1.pxr | 5 +- 7 files changed, 189 insertions(+), 96 deletions(-) diff --git a/praxiscore-api/src/main/java/org/praxislive/core/types/PError.java b/praxiscore-api/src/main/java/org/praxislive/core/types/PError.java index a45c6c8f..4d25973f 100644 --- a/praxiscore-api/src/main/java/org/praxislive/core/types/PError.java +++ b/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 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 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 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() { 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 type - = (Class) 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 type - = (Class) 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 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 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 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 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); + } + } diff --git a/praxiscore-api/src/main/java/org/praxislive/core/types/PMap.java b/praxiscore-api/src/main/java/org/praxislive/core/types/PMap.java index bc0fc9a6..ea929c02 100644 --- a/praxiscore-api/src/main/java/org/praxislive/core/types/PMap.java +++ b/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 from(Value value) { try { diff --git a/praxiscore-hub-net/src/main/java/org/praxislive/hub/net/IonCodec.java b/praxiscore-hub-net/src/main/java/org/praxislive/hub/net/IonCodec.java index 9423cf61..be9d1e51 100644 --- a/praxiscore-hub-net/src/main/java/org/praxislive/hub/net/IonCodec.java +++ b/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()) { diff --git a/praxiscore-hub-net/src/test/java/org/praxislive/hub/net/IonCodecTest.java b/praxiscore-hub-net/src/test/java/org/praxislive/hub/net/IonCodecTest.java index fa556d98..4fd6f7ba 100644 --- a/praxiscore-hub-net/src/test/java/org/praxislive/hub/net/IonCodecTest.java +++ b/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()); } diff --git a/praxiscore-launcher/src/main/java/org/praxislive/launcher/LogServiceImpl.java b/praxiscore-launcher/src/main/java/org/praxislive/launcher/LogServiceImpl.java index bcbefbac..f46bfc15 100644 --- a/praxiscore-launcher/src/main/java/org/praxislive/launcher/LogServiceImpl.java +++ b/praxiscore-launcher/src/main/java/org/praxislive/launcher/LogServiceImpl.java @@ -1,7 +1,7 @@ /* * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. * - * Copyright 2021 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 @@ -20,11 +20,8 @@ * have any questions. * */ - package org.praxislive.launcher; -import java.io.PrintWriter; -import java.io.StringWriter; import java.util.List; import java.util.Objects; import org.praxislive.base.AbstractRoot; @@ -37,9 +34,11 @@ import org.praxislive.core.types.PError; class LogServiceImpl extends AbstractRoot implements RootHub.ServiceProvider { - + + private static final String NEWLINE = System.lineSeparator(); + private final LogLevel logLevel; - + LogServiceImpl(LogLevel logLevel) { this.logLevel = Objects.requireNonNull(logLevel); } @@ -62,29 +61,29 @@ protected void processCall(Call call, PacketRouter router) { } } } - + private void processLog(Call call) throws Exception { var src = call.from().component(); var args = call.args(); + var sb = new StringBuilder(); for (int i = 1; i < args.size(); i += 2) { var level = LogLevel.valueOf(args.get(i - 1).toString()); if (!logLevel.isLoggable(level)) { continue; } var arg = args.get(i); - var sw = new StringWriter(); - var pw = new PrintWriter(sw); - pw.append(level.name()).append(" : ").append(src.toString()).println(); - if (arg instanceof PError) { - var err = (PError) arg; - pw.append(err.exceptionType().getSimpleName()).append(" - "); - pw.append(err.message()).append("\n"); - err.exception().ifPresent(ex -> ex.printStackTrace(pw)); - } else { - pw.append(arg.toString()).println(); - } - pw.flush(); - System.err.println(sw.toString()); + sb.append(level.name()).append(" : ").append(src.toString()).append(NEWLINE); + PError.from(arg).ifPresentOrElse(err -> { + sb.append(err.errorType()).append(" - ").append(err.message()).append(NEWLINE); + var stack = err.stackTrace(); + if (!stack.isBlank()) { + sb.append(stack).append(NEWLINE); + } + }, () -> { + sb.append(arg.toString()).append(NEWLINE); + }); + System.err.println(sb.toString()); + sb.setLength(0); } System.err.flush(); } diff --git a/praxiscore-script/src/main/java/org/praxislive/script/ScriptStackFrame.java b/praxiscore-script/src/main/java/org/praxislive/script/ScriptStackFrame.java index 62970469..36ccc79f 100644 --- a/praxiscore-script/src/main/java/org/praxislive/script/ScriptStackFrame.java +++ b/praxiscore-script/src/main/java/org/praxislive/script/ScriptStackFrame.java @@ -186,7 +186,7 @@ private PArray addErrorToTrap(PArray trap, Value response) { msg = activeCommand + " : Error"; } else { msg = PError.from(response) - .map(err -> activeCommand + " : " + err.exceptionType().getSimpleName() + .map(err -> activeCommand + " : " + err.errorType() + " : " + err.message()) .orElse(activeCommand + " : Error : " + response); } diff --git a/testsuite/tests/core/async-functions/data1.pxr b/testsuite/tests/core/async-functions/data1.pxr index f7559983..9d24c036 100644 --- a/testsuite/tests/core/async-functions/data1.pxr +++ b/testsuite/tests/core/async-functions/data1.pxr @@ -176,10 +176,9 @@ import org.praxislive.core.services.SystemManagerService; + response.result().args() ); error.send(); - \} else if (response.error().exceptionType() - != UnsupportedOperationException.class) \{ + \} else if (!\"UnsupportedOperationException\".equals(response.error().errorType())) \{ log(ERROR, \" wrong exception received : \" - + response.error().exceptionType()); + + response.error().errorType()); error.send(); \} else \{ log(INFO, \"Test 4 : OK\");