write trace to file as it comes

[pelikhan]

The changes introduced in the GIT_DIFF are primarily related to the management of tracing in the codebase.

• 📖 The function runScript appears to be handling loading scripts and preserving trace information.
• 🚀 Added code to ensure the "out" and "outTrace" directories exist (ensureDir), and to clean up the “out” directory is if the removeOut flag is set (emptyDir).
• ✨ Introduced a new event listener to TRACE_CHUNK event in the trace object. When this event fires, the trace chunk details are appended to the file specified by outTrace.
• 👥 The same TRACE_CHUNK listener logic is copied for a new file "res.trace.md" set in the "out" directory, barring circumstances where the outTrace file is the same as "res.trace.md".
• 🗑️ The subsequent code which wrote the full trace content to the outTrace file at the end of the function (writeText(outTrace, trace.content)) has now been removed to reflect the new approach.
• 🔄 This indicates a shift from a full dump of tracing at the end of function execution towards a real-time tracing information write process. This would benefit programs where live trace information is needed during script running, or where large traces could benefit from being written incrementally.

generated by pr-describe

[github-actions]

The changes in the GIT_DIFF primarily revolve around how traces are handled and how output directories are managed.

Notably, changes include:

• Importing dirname from node:path and writeFile, appendFile from fs/promises.
• Adding an event listener to the trace object that responds to TRACE_CHUNK events. When these events occur, the trace chunk is appended to the output trace file.
• Adding similar functionality for an output file named res.trace.md in the output directory.
• Making sure the output directory is created and cleaned (if necessary) before the tracing functionality is started.
• Removing the previously used writeText(outTrace, trace.content) that wrote the entire trace content at once.

Concerns:

• The trace writing functionality is duplicated for outTrace and res.trace.md. It would be better to abstract this into a separate function that handles trace writing given a file name and a trace object, thus not repeating code.
• The code checks if (out && trace) twice, once for outTrace and once for res.trace.md. Consolidating these might make the code easier to follow.

Suggested changes:

+ async function setupTraceWriting(trace: Trace, filename: string) {
+     await ensureDir(dirname(filename))
+     await writeFile(filename, "", { encoding: "utf-8" })
+     trace.addEventListener(
+         TRACE_CHUNK,
+         async (ev) => {
+             const tev = ev as TraceChunkEvent
+             await appendFile(filename, tev.chunk, { encoding: "utf-8" })
+         },
+         false
+     )
+ }

+ if (out) {
+     if (removeOut) await emptyDir(out)
+     await ensureDir(out)
+ }
+ if (out && trace) {
+     const ofn = join(out, "res.trace.md")
+     if (outTrace) {
+         setupTraceWriting(trace, outTrace);
+     }
+     if (ofn !== outTrace) {
+         setupTraceWriting(trace, ofn);
+     }
+ }

Overall, the changes look fine except for the noted concerns and suggested improvements. The change prioritizes writing trace chunks as they arrive instead of at the end, which could be beneficial for long-running processes. However, make sure to test these changes thoroughly to ensure that the trace writing functionality works as expected.

Concerns aside, LGTM 🚀

generated by pr-review

[github-actions]

Using an async function as an event listener can lead to unhandled promise rejections if the promise is rejected. Consider wrapping the async function call within the event listener in a try-catch block to handle any potential errors. 🛠️

generated by pr-review-commit async_in_event_listener

[github-actions]

The setup for trace writing is only done if 'out' and 'trace' are truthy. However, 'outTrace' is used later in the code without checking if it was set up correctly. This could potentially lead to errors if 'outTrace' was not set up because 'out' or 'trace' were falsy. Please ensure that 'outTrace' is always correctly set up. 🧐

generated by pr-review-commit conditional_trace_setup

[github-actions]

The line of code that writes the trace content to 'outTrace' has been removed. If 'outTrace' is expected to contain the trace content, this could lead to unexpected behavior. Please ensure that the trace content is written to 'outTrace' as expected. 🕵️‍♀️

generated by pr-review-commit removed_trace_write