gnodet-bot commented on code in PR #13695:
URL: https://github.com/apache/maven/pull/13695#discussion_r4368844990


##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -190,52 +201,72 @@ protected int doInvoke(C context) throws Exception {
     }
 
     protected InvokerException.ExitException handleException(C context, 
Exception e) {
-        printErrors(
-                context,
-                context.options().showErrors().orElse(false),
-                List.of(new Logger.Entry(Logger.Level.ERROR, e.getMessage(), 
e.getCause())),
-                context.logger);
+        printError(context, context.options().showErrors().orElse(false), 
e.getMessage(), e.getCause());
         return new InvokerException.ExitException(2);
     }
 
-    protected void printErrors(C context, boolean showStackTrace, 
List<Logger.Entry> entries, Logger logger) {
-        // if accumulating logger passed, this is "early failure", swap logger 
for stdErr and use that to emit log
-        if (logger instanceof AccumulatingLogger) {
-            logger = new 
SystemLogger(context.invokerRequest.stdErr().orElse(null));
-        }
-        // this is important message; many Maven IT assert for presence of 
this message
-        logger.error("Error executing " + 
context.invokerRequest.parserRequest().commandName() + ".");
-        for (Logger.Entry entry : entries) {
-            if (showStackTrace) {
-                logger.log(entry.level(), entry.message(), entry.error());
+    protected void printError(C context, boolean showStackTrace, String 
message, Throwable error) {
+        if (context.loggingActivated) {
+            // Post-activation: route through the logging API so output honors 
-l (log file) redirection.
+            context.logger.error(
+                    "Error executing " + 
context.invokerRequest.parserRequest().commandName() + ".");
+            if (showStackTrace && error != null) {
+                context.logger.error(message != null ? message : "", error);
             } else {
-                logger.error(entry.message());
-                for (Throwable cause = entry.error();
-                        cause != null && cause != cause.getCause();
-                        cause = cause.getCause()) {
-                    logger.log(entry.level(), "Caused by: " + 
cause.getMessage());
+                context.logger.error(message != null ? message : "");
+                for (Throwable cause = error; cause != null && cause != 
cause.getCause(); cause = cause.getCause()) {
+                    context.logger.error("Caused by: " + (cause.getMessage() 
!= null ? cause.getMessage() : ""));
                 }
             }
+        } else {
+            // Pre-activation: the logging infrastructure is not fully set up. 
Before activateLogging() installs
+            // the real sink, context.logger routes events into 
AccumulatingLogSink; those events are lost when
+            // the exception short-circuits activateLogging(). Write to stderr 
directly for guaranteed visibility.
+            PrintWriter err = new 
PrintWriter(context.invokerRequest.stdErr().orElse(System.err), true);
+            err.println("[ERROR] Error executing "
+                    + context.invokerRequest.parserRequest().commandName() + 
".");
+            if (showStackTrace && error != null) {
+                err.println("[ERROR] " + (message != null ? message : ""));
+                error.printStackTrace(err);
+            } else {
+                err.println("[ERROR] " + (message != null ? message : ""));
+                for (Throwable cause = error; cause != null && cause != 
cause.getCause(); cause = cause.getCause()) {
+                    err.println("[ERROR] Caused by: " + (cause.getMessage() != 
null ? cause.getMessage() : ""));
+                }
+            }
+            // Drain any buffered bootstrap events so diagnostic messages 
emitted before the failure are visible.
+            LogSink currentSink = LoggerManager.get().getLogSink();
+            if (currentSink != null) {
+                currentSink.drainTo(event -> {
+                    err.println("[" + event.level() + "] " + event.message());

Review Comment:
   🟡 **`[WARN]` vs `[WARNING]` — inconsistent level format in bootstrap drain 
output.**
   
   `event.level()` calls `LogLevel.WARN.name()` → `"WARN"`, so this produces 
`[WARN] <message>`. But Maven's standard console format is `[WARNING]` — see 
`SimpleBuildEventListener.renderLevel()` which explicitly maps `WARN` → 
`"WARNING"`. A user reading the terminal or a `-l` log file will see `[WARN]` 
during bootstrap drain and `[WARNING]` everywhere else.
   
   Same issue on line 264 (the `validate()` drain path).
   
   ```suggestion
                       err.println("[" + (event.level() == 
org.apache.maven.api.logging.LogLevel.WARN ? "WARNING" : event.level().name()) 
+ "] " + event.message());
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to