Skip to content

SED-4789 Implement live reporting in the Node.js agent #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jeromecomte wants to merge 3 commits into
base: master
Choose a base branch
from
Open

SED-4789 Implement live reporting in the Node.js agent #661

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45da24f
SED-4789 Implement live reporting in the Node.js agent
jeromecomte Jun 22, 2026
3860b47
SED-4789 PR feedbacks
jeromecomte Jun 24, 2026
d718806
SED-4789 PR feedback: splitting live-reporting.js
jeromecomte Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion step-node/step-node-agent/api/controllers/agent-fork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

const { OutputBuilder } = require("./output");
const { createLiveReporting } = require("./live-reporting");
const Session = require("./session");
const fs = require("fs");
const path = require('path')
Expand All @@ -34,6 +35,7 @@ process.on('message', async ({ type, projectPath, functionName, input, propertie
pendingUncaughtException = null;
console.log("[Agent fork] Calling keyword " + functionName)
const outputBuilder = new OutputBuilder();
const liveReporting = createLiveReporting(properties);
try {
Comment on lines 37 to 39

@gemini-code-assist gemini-code-assist Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To prevent any unexpected errors during createLiveReporting initialization from crashing the fork process, declare liveReporting using let and initialize it inside the try block. This ensures any initialization errors are caught and reported gracefully via outputBuilder.fail.

    const outputBuilder = new OutputBuilder();
    let liveReporting;
    try {
      liveReporting = createLiveReporting(properties);

@jeromecomte jeromecomte Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — liveReporting is now declared with let and initialized as the first statement inside the try, so any initialization error is reported via outputBuilder.fail instead of escaping the message handler.

if (!keywordDirectoryExists(projectPath, keywordDirectory)) {
outputBuilder.fail("The keyword directory '" + keywordDirectory + "' doesn't exist in " + path.basename(projectPath) + ". Possible cause: If using TypeScript, the keywords may not have been compiled. Fix: Ensure your project is built before deploying to Step or during 'npm install'.")
Expand All @@ -52,7 +54,7 @@ process.on('message', async ({ type, projectPath, functionName, input, propertie
if(beforeKeyword) {
await beforeKeyword(functionName);
}
await keyword(input, outputBuilder, session, properties);
await keyword(input, outputBuilder, session, properties, liveReporting);
} catch (e) {
console.log("[Agent fork] Keyword execution failed with following error", e)
const onError = module['onError'];
Expand Down Expand Up @@ -82,6 +84,12 @@ process.on('message', async ({ type, projectPath, functionName, input, propertie
// Flush the event loop so unhandledRejection / uncaughtException from the keyword
// (e.g. fire-and-forget promises, nextTick throws) land before we send the result.
await new Promise(resolve => setImmediate(resolve));
// Close live reporting: flushes any buffered measures and waits for in-flight uploads.
try {
await liveReporting.close();
} catch (e) {
console.log("[Agent fork] Error while closing live reporting", e);
}
Comment on lines +92 to +98

@gemini-code-assist gemini-code-assist Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Add a safety check to ensure liveReporting is defined before attempting to call close(), preventing a TypeError if initialization failed or was skipped.

      try {
        if (liveReporting) {
          await liveReporting.close();
        }
      } catch (e) {
        console.log("[Agent fork] Error while closing live reporting", e);
      }
References
  1. In the agent forker process (agent-fork.js), use console.log for logging as the main logger utility is not available.

@jeromecomte jeromecomte Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — added the if (liveReporting) guard before await liveReporting.close().

// Surface inter-keyword errors first, labelled clearly as coming from a previous keyword.
if (prevUnhandledRejection) {
const sep = outputBuilder.hasError() ? '\n' : '';
Expand Down
17 changes: 16 additions & 1 deletion step-node/step-node-agent/api/controllers/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ const logger = require('../logger').child({ component: 'Agent' });

const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';

// Resolve the bundled `ws` module path once, so it can be injected into forked keyword processes
// (whose own require() resolves against the keyword project, not the agent). Used by live reporting
// for streaming file uploads. If ws is missing, file uploads degrade to discarding.
let wsModulePath = null;
try {
wsModulePath = require.resolve('ws');
} catch {
logger.warn('The ws module could not be resolved; live reporting file uploads will be disabled');
}

process.on('unhandledRejection', error => {
logger.error('Critical: an unhandled error (unhandled promise rejection) occurred and might not have been reported:', error)
})
Expand Down Expand Up @@ -348,10 +358,15 @@ class ForkedAgent {
fs.mkdirSync(agentForkerLibPath, { recursive: true });
fs.copyFileSync(path.resolve(__dirname, 'agent-fork.js'), path.join(agentForkerLibPath, 'agent-fork.js'));
fs.copyFileSync(path.join(__dirname, 'output.js'), path.join(agentForkerLibPath, 'output.js'));
fs.copyFileSync(path.join(__dirname, 'live-reporting.js'), path.join(agentForkerLibPath, 'live-reporting.js'));

@david-stephan david-stephan Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is the reason of the "big" live-reporting.js file, but for code review and maintainability it would have been nicer to split the code.

@jeromecomte jeromecomte Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with this. I would also prefer to split the code in dedicated files. In theory we could already do it and simply add more files here but I would prefer to have a better solution than the copy first

@jeromecomte jeromecomte Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — live-reporting.js is now split into a live-reporting/ folder: index.js (container + factory + public API), shared.js (logger, reporting-URL resolution, the batching REST poster), measures.js, metrics.js, and file-uploads.js. To avoid maintaining a per-file copy list in agent.js, the agent now copies the whole folder recursively (fs.cpSync(..., { recursive: true })), so adding new modules no longer requires touching agent.js.

fs.copyFileSync(path.join(__dirname, 'session.js'), path.join(agentForkerLibPath, 'session.js'));
this.agentForkerLibPath = agentForkerLibPath;
this.startupChunks = [];
this.forkProcess = fork(path.join(agentForkerLibPath, 'agent-fork.js'), [], {cwd: keywordProjectPath, silent: true});
const forkEnv = { ...process.env };
if (wsModulePath) {
forkEnv.STEP_AGENT_WS_MODULE = wsModulePath;
}
this.forkProcess = fork(path.join(agentForkerLibPath, 'agent-fork.js'), [], {cwd: keywordProjectPath, silent: true, env: forkEnv});
// Capture stdout/stderr immediately so startup crashes are not silently lost
if (this.forkProcess.stdout) {
this.forkProcess.stdout.on('data', (data) => this.startupChunks.push(data));
Expand Down
Loading