oscerd commented on code in PR #27399:
URL: https://github.com/apache/camel/pull/27399#discussion_r4193672692
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -218,6 +238,61 @@ private static RuntimeHelper.ProcessInfo findByPid(long
pid) {
return null;
}
+ /**
+ * The console output of a run that failed to start, as camel_get_log
shows a log (CAMEL-25364): each record keeps
+ * its message, an error its root cause and the first line of the user's
code, and the stack frames are left out.
+ * The end of the raw output is mostly frames of the runtime, and the line
that says what went wrong is above them.
+ */
+ static String failureOutput(Path output) {
+ List<String> lines;
+ try {
+ lines = Files.readAllLines(output, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ return readTail(output);
+ }
+ if (lines.size() > 5000) {
+ lines = lines.subList(lines.size() - 5000, lines.size());
+ }
+ lines = lines.stream().filter(l -> !l.isBlank()).toList();
+ JsonObject log = LogFileReader.build(lines, FAILURE_RECORDS, null,
null, false, new JsonObject());
Review Comment:
**Multi-line exception messages (e.g. javac diagnostics) are dropped with
the stack trace**
`failureOutput` folds with `details=false`, and `LogFileReader.withoutTrace`
then removes the whole `detail` of every record whose continuation lines
contain a frame, keeping the head line, `cause` (the first line of the root
cause, only when the head does not already contain it) and `at`. When the
exception message itself spans several lines, the cause is lost. That includes
the case this issue is about, an added Java class that does not compile:
`MultiCompile` throws `new ReflectException("Compilation error:\n" + sj + "\n"
+ out)`, and `KameletMain` logs it as `LOG.error("Error starting Camel: {}", e,
e)`. The head line ends at `Compilation error:`, and the javac diagnostics
(`OrderNumber.java:12: error: cannot find symbol`, ...) are continuation lines
in the same block as the `\tat` frames, so the result is only:
```
ERROR Error starting Camel: org.joor.ReflectException: Compilation error:
(N lines of stack trace left out)
```
Unlike `camel_get_log` (which has `details=true`), here the folded text is
the only copy the agent gets: `Run` deletes `~/.camel/<pid>.log` on exit, and
the path of the capture is not returned.
Relatedly, `readTail` only runs when the output is blank, so non-empty
output without log records becomes one record headed by its first line, capped
at 20 detail lines; the end, where a launcher prints its error, is cut, which
does not match "Output with no log records is returned as it is".
Keeping the non-frame continuation lines (e.g. drop only the frame and `...
N more` lines before folding, or keep each exception's lines up to its first
frame), falling back to `readTail` when no line matched the log pattern, and a
test fixture with a multi-line exception message would cover it.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -72,6 +76,22 @@ public static JsonObject run(Path directory, List<String>
files, String name, bo
return result;
}
result.put("directory", directory.toString());
+ if (sourceDir) {
+ // the whole directory is the app: one already running from it is
that app, and a second copy only fights
+ // it for ports and files and makes its name ambiguous to the
other tools
+ RuntimeHelper.ProcessInfo running = runningFrom(directory);
+ if (running != null) {
+ String runningName = running.contextName() != null &&
!running.contextName().isBlank()
+ ? running.contextName() : running.name();
+ result.put("status", "running");
+ result.put("pid", running.pid());
+ result.put("name", runningName);
+ result.put("message", runningName + " (pid " + running.pid() +
") already runs from this directory;"
+ + " a second copy was not started. In
dev mode a changed or added file is reloaded;"
+ + " camel_control restart restarts it,
for example after adding a Java class.");
Review Comment:
**The reload/restart advice does not hold for an app started with explicit
files**
The guard returns any process whose working directory is this directory,
which may be one started earlier with explicit files (by `camel_run` with
`files`, or a user's `camel run a.camel.yaml`) or without `--dev`. For an
explicit-files run, dev mode only watches the listed files and `camel_control
restart` replays the same command line, so neither picks up an added Java
class, yet the message promises both. `restart()` already reads
`ProcessHandle.info().arguments()`, so the guard could treat the process as the
app only when it was started with `--source-dir`, report dev mode from `--dev`,
and otherwise say that an integration started with explicit files runs there.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -72,6 +76,22 @@ public static JsonObject run(Path directory, List<String>
files, String name, bo
return result;
}
result.put("directory", directory.toString());
+ if (sourceDir) {
+ // the whole directory is the app: one already running from it is
that app, and a second copy only fights
+ // it for ports and files and makes its name ambiguous to the
other tools
+ RuntimeHelper.ProcessInfo running = runningFrom(directory);
Review Comment:
**A copy that is still starting, or launched by a concurrent call, is
invisible to the guard**
`runningFrom` only sees processes that have already written
`<pid>-status.json`. So after `run()` returns `status: starting` at the startup
timeout (typical on a first run, while dependencies download), a second
`camel_run` starts a second copy, and so do two calls in flight at the same
time. Remembering what this launcher started (e.g. a map from the directory to
the `ProcessHandle`, checked with `isAlive()`) alongside the status-file lookup
would cover that window.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -218,6 +238,61 @@ private static RuntimeHelper.ProcessInfo findByPid(long
pid) {
return null;
}
+ /**
+ * The console output of a run that failed to start, as camel_get_log
shows a log (CAMEL-25364): each record keeps
+ * its message, an error its root cause and the first line of the user's
code, and the stack frames are left out.
+ * The end of the raw output is mostly frames of the runtime, and the line
that says what went wrong is above them.
+ */
+ static String failureOutput(Path output) {
+ List<String> lines;
+ try {
+ lines = Files.readAllLines(output, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ return readTail(output);
+ }
+ if (lines.size() > 5000) {
+ lines = lines.subList(lines.size() - 5000, lines.size());
+ }
+ lines = lines.stream().filter(l -> !l.isBlank()).toList();
+ JsonObject log = LogFileReader.build(lines, FAILURE_RECORDS, null,
null, false, new JsonObject());
+ JsonArray rows = (JsonArray) log.get("lines");
+ if (rows == null || rows.isEmpty()) {
+ return readTail(output);
+ }
+ StringBuilder sb = new StringBuilder();
+ // newest first in the log tool; oldest first here, so it reads as the
console did
+ for (int i = rows.size() - 1; i >= 0; i--) {
+ JsonObject r = (JsonObject) rows.get(i);
+ String level = r.getStringOrDefault("time", "").isEmpty() ? "" :
r.getStringOrDefault("level", "") + " ";
+ sb.append(level).append(r.getStringOrDefault("message", ""));
+ if (r.get("cause") != null) {
+ sb.append("\n cause: ").append(r.getString("cause"));
+ }
+ if (r.get("at") != null) {
+ sb.append("\n at: ").append(r.getString("at"));
+ }
+ if (r.get("detailLines") != null) {
+ sb.append("\n (").append(r.get("detailLines")).append(" lines
of stack trace left out)");
+ } else if (r.get("detail") != null) {
+ sb.append("\n").append(r.getString("detail"));
+ }
+ sb.append("\n");
+ }
+ return sb.toString().strip();
+ }
+
+ /** The integration that runs with this directory as its working
directory, or null. */
+ private static RuntimeHelper.ProcessInfo runningFrom(Path directory) {
+ Path wanted = directory.toAbsolutePath().normalize();
+ for (RuntimeHelper.ProcessInfo p : RuntimeHelper.discoverProcesses()) {
+ Path dir = workingDirectory(p.pid());
+ if (dir != null &&
dir.toAbsolutePath().normalize().equals(wanted)) {
Review Comment:
**The guard misses the running app when the directory path goes through a
symlink**
`runningFrom` compares `toAbsolutePath().normalize()` of two paths from
different sources: `runtime.directory` is the child's `new
File(".").getAbsolutePath()`, i.e. its symlink-resolved working directory,
while the caller's directory only goes through `toAbsolutePath().normalize()`.
When the agent passes a path through a symlink (every project under `/tmp` or
`$TMPDIR` on macOS), the guard never matches and a second copy is started. Both
paths are existing directories here, so `Files.isSameFile(dir, wanted)`
(falling back to the current comparison on `IOException`) or comparing
`toRealPath()` would fix it.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -218,6 +238,61 @@ private static RuntimeHelper.ProcessInfo findByPid(long
pid) {
return null;
}
+ /**
+ * The console output of a run that failed to start, as camel_get_log
shows a log (CAMEL-25364): each record keeps
+ * its message, an error its root cause and the first line of the user's
code, and the stack frames are left out.
+ * The end of the raw output is mostly frames of the runtime, and the line
that says what went wrong is above them.
+ */
+ static String failureOutput(Path output) {
+ List<String> lines;
+ try {
+ lines = Files.readAllLines(output, StandardCharsets.UTF_8);
Review Comment:
**Strict UTF-8 decoding: a single non-UTF-8 byte makes the failure output
empty**
`Files.readAllLines(output, UTF_8)` throws `MalformedInputException` on any
malformed byte, and `readTail`'s `Files.readString(output, UTF_8)` throws the
same, so the output comes back empty. Console output is not always UTF-8 (a
redirected stdout on Windows uses the native code page, so `C:\Users\Jörg\...`
is enough). `LogFileReader.tail` already decodes leniently; doing the same here
(and in `readTail`) keeps the output:
```suggestion
lines = new String(Files.readAllBytes(output),
StandardCharsets.UTF_8).lines().toList();
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IntegrationLauncher.java:
##########
@@ -72,6 +76,22 @@ public static JsonObject run(Path directory, List<String>
files, String name, bo
return result;
}
result.put("directory", directory.toString());
+ if (sourceDir) {
+ // the whole directory is the app: one already running from it is
that app, and a second copy only fights
+ // it for ports and files and makes its name ambiguous to the
other tools
+ RuntimeHelper.ProcessInfo running = runningFrom(directory);
+ if (running != null) {
+ String runningName = running.contextName() != null &&
!running.contextName().isBlank()
+ ? running.contextName() : running.name();
+ result.put("status", "running");
+ result.put("pid", running.pid());
+ result.put("name", runningName);
Review Comment:
**The 'running' result has no log file**
Both `camel_run` descriptions promise the log file ("Returns the pid and log
file"), and the `started` result carries `log`, but the new `running` result
does not:
```suggestion
result.put("name", runningName);
result.put("log", LogFileReader.logFile(running.pid(),
running.name()).toString());
```
--
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]
