Skip to content
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ab71414
wip
Pijukatel Jul 24, 2025
d328ada
More wip, todo figure out streaming buffer
Pijukatel Aug 11, 2025
a8ae55f
WIP, polish the tests. Add those that exists in Python version.
Pijukatel Aug 18, 2025
4f2146d
Do I have to split logger into formater + logger?
Pijukatel Aug 25, 2025
cc7aba9
Some working draft
Pijukatel Oct 21, 2025
50b2732
Try to exclude files, to make it possible to install/build from branch
Pijukatel Oct 21, 2025
f25935c
Add log option to ActorClient.call
Pijukatel Oct 22, 2025
4aa0125
Update type hint
Pijukatel Oct 22, 2025
c410190
Format and lint and add test
Pijukatel Oct 22, 2025
d9a900a
Merge remote-tracking branch 'origin/master' into log-redirection
Pijukatel Oct 22, 2025
1c293da
Add Actor call test, 3 scenarios
Pijukatel Oct 23, 2025
0456c20
Add more comments
Pijukatel Oct 23, 2025
dc88c2b
Remove unused import
Pijukatel Oct 23, 2025
ec0d60e
Fix failing tests
Pijukatel Oct 23, 2025
fc7bb6c
Do not run log redirection when in Browser
Pijukatel Oct 23, 2025
bbeeee4
Review comments
Pijukatel Oct 27, 2025
00bf7ea
Update tests to pas on Node 16
Pijukatel Oct 27, 2025
5fd0fc7
Properlyhnadle multibyte characters that are split by chunking
Pijukatel Oct 29, 2025
5d4c1f4
Fix awaiting logs without calling run
Pijukatel Oct 29, 2025
51dd246
Make it possible to set response sequence per each test if needed
Pijukatel Oct 29, 2025
71d7fb5
Review comments
Pijukatel Nov 3, 2025
0b2e6e6
Simplify handling of multibyte chars
Pijukatel Nov 4, 2025
0440e39
Basic review feedback
Pijukatel Nov 5, 2025
0b84a32
Simplify loging and add more docs
Pijukatel Nov 5, 2025
bca27b2
Cleanup using finally chain
Pijukatel Nov 5, 2025
8eb6e71
Use setTimeout from node
Pijukatel Nov 6, 2025
dfa5dcd
Review comments
Pijukatel Nov 11, 2025
3e48c60
Add `default` alternative to `undefined`
Pijukatel Nov 11, 2025
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
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@
"url": "https://github.com/apify/apify-client-js/issues"
},
"homepage": "https://docs.apify.com/api/client/js/",
"files": [
Copy link
Author

Choose a reason for hiding this comment

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

Just temp for testing

Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what are you testing this way?

Copy link
Author

@Pijukatel Pijukatel Nov 5, 2025

Choose a reason for hiding this comment

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

I have an Actor that just points to this branch and I can test the tip of it on the Apify platform as it compiles on the fly. It takes time, but it does not require me to do any manual actions.

The actor has in pacakge.json

	"dependencies": {
        "apify-client": "github:apify/apify-client-js#log-redirection",
        ...
	},
	"scripts": {
        ...
        "postinstall": "cd node_modules/apify-client && npm install && npm run build:node"
	},

...
    "overrides": {
        "apify-client": "github:apify/apify-client-js#log-redirection"
    },

and I removed --omit=dev from npm install in the docker file

If there is a more convenient way for TypeScript-based code, I would be very happy to learn about it.

"dist",
"!dist/*.tsbuildinfo"
],
"scripts": {
"build": "npm run clean && npm run build:node && npm run build:browser",
"postbuild": "gen-esm-wrapper dist/index.js dist/index.mjs",
Expand All @@ -67,6 +63,7 @@
"@apify/utilities": "^2.18.0",
"@crawlee/types": "^3.3.0",
"agentkeepalive": "^4.2.1",
"ansi-colors": "^4.1.1",
"async-retry": "^1.3.3",
"axios": "^1.6.7",
"content-type": "^1.0.5",
Expand Down
26 changes: 24 additions & 2 deletions src/resource_clients/actor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import ow from 'ow';

import type { RUN_GENERAL_ACCESS } from '@apify/consts';
import { ACT_JOB_STATUSES, ACTOR_PERMISSION_LEVEL, META_ORIGINS } from '@apify/consts';
import { Log } from '@apify/log';

import type { ApiClientSubResourceOptions } from '../base/api_client';
import { ResourceClient } from '../base/resource_client';
Expand All @@ -13,6 +14,7 @@ import { ActorVersionCollectionClient } from './actor_version_collection';
import type { Build, BuildClientGetOptions } from './build';
import { BuildClient } from './build';
import { BuildCollectionClient } from './build_collection';
import type { StreamedLog } from './log';
import { RunClient } from './run';
import { RunCollectionClient } from './run_collection';
import type { WebhookUpdateData } from './webhook';
Expand Down Expand Up @@ -139,18 +141,37 @@ export class ActorClient extends ResourceClient {
webhooks: ow.optional.array.ofType(ow.object),
maxItems: ow.optional.number.not.negative,
maxTotalChargeUsd: ow.optional.number.not.negative,
log: ow.optional.any(ow.null, ow.object.instanceOf(Log)),
restartOnError: ow.optional.boolean,
forcePermissionLevel: ow.optional.string.oneOf(Object.values(ACTOR_PERMISSION_LEVEL)),
}),
);

const { waitSecs, ...startOptions } = options;
const { waitSecs, log = 'default', ...startOptions } = options;
const { id } = await this.start(input, startOptions);

// Calling root client because we need access to top level API.
// Creating a new instance of RunClient here would only allow
// setting it up as a nested route under actor API.
return this.apifyClient.run(id).waitForFinish({ waitSecs });
const newRunClient = this.apifyClient.run(id);

if (!log) {
return newRunClient.waitForFinish({ waitSecs });
}
let streamedLog: StreamedLog;

if (log === 'default') {
streamedLog = await newRunClient.getStreamedLog();
} else {
streamedLog = await newRunClient.getStreamedLog({ toLog: log });
}

try {
await streamedLog.start();
return this.apifyClient.run(id).waitForFinish({ waitSecs });
} finally {
await streamedLog.stop();
}
}

/**
Expand Down Expand Up @@ -426,6 +447,7 @@ export interface ActorStartOptions {

export interface ActorCallOptions extends Omit<ActorStartOptions, 'waitForFinish'> {
waitSecs?: number;
log?: Log | null;
Copy link
Author

Choose a reason for hiding this comment

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

The difference from Python is the name of the argument. While in Python it is called logger and can be of type Logger, here it is log and can be of type Log.

In JS tooling, the Log is a kind of Logger from Python, as it (implicitly) includes level, formatting, and handler.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for accepting null here? Wouldn't log?: Log work as well? If there are any semantics that I'm failing to see, please document them.

Copy link
Author

@Pijukatel Pijukatel Nov 5, 2025

Choose a reason for hiding this comment

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

undefined -> use default
null -> explicitly requesting not to use any redirect logger
Log -> use whatever log is passed

In Python, it is similar, but since there is no undefined, there it is like
"default" -> use default
None -> explicitly requesting not to use any redirect logger
Logger -> use whatever Logger is passed

Added description

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't using a string literal like in Python work as well? I'd consider that more readable, too.

Copy link
Author

Choose a reason for hiding this comment

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

Isn't that going to cause more confusion? Aren't the default values in JS commonly applied when the key is undefined in options?

Using default would allow to define the default value through both undefined and default literal

Copy link
Author

Choose a reason for hiding this comment

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

I tried in the last commit. It feels very redundant to me, but I can live with it
3e48c60

}

export interface ActorRunListItem {
Expand Down
192 changes: 189 additions & 3 deletions src/resource_clients/log.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
// eslint-disable-next-line max-classes-per-file
import type { Readable } from 'node:stream';

import c from 'ansi-colors';

import type { Log } from '@apify/log';
import { Logger, LogLevel } from '@apify/log';

import type { ApifyApiError } from '../apify_api_error';
import type { ApiClientSubResourceOptions } from '../base/api_client';
import { ResourceClient } from '../base/resource_client';
Expand All @@ -20,11 +26,11 @@ export class LogClient extends ResourceClient {
/**
* https://docs.apify.com/api/v2#/reference/logs/log/get-log
*/
async get(): Promise<string | undefined> {
async get(options: LogOptions = {}): Promise<string | undefined> {
const requestOpts: ApifyRequestConfig = {
url: this._url(),
method: 'GET',
params: this._params(),
params: this._params(options),
};

try {
Expand All @@ -41,9 +47,10 @@ export class LogClient extends ResourceClient {
* Gets the log in a Readable stream format. Only works in Node.js.
* https://docs.apify.com/api/v2#/reference/logs/log/get-log
*/
async stream(): Promise<Readable | undefined> {
async stream(options: LogOptions = {}): Promise<Readable | undefined> {
const params = {
stream: true,
raw: options.raw,
};

const requestOpts: ApifyRequestConfig = {
Expand All @@ -63,3 +70,182 @@ export class LogClient extends ResourceClient {
return undefined;
}
}

export interface LogOptions {
/** @default false */
raw?: boolean;
}

// Temp create it here and ask Martin where to put it
Copy link
Member

Choose a reason for hiding this comment

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

this is where the apify logger is defined, so maybe there?

https://github.com/apify/apify-shared-js/tree/master/packages/log

it doesnt feel right to have this in the client

Copy link
Author

Choose a reason for hiding this comment

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

Not sure about that. I can't imagine a scenario where this would be used outside of the client. It is very client-specific code. Maybe I can put it in to separate file? Something like redirection_utils or logging_utils?


const DEFAULT_OPTIONS = {
/** Whether to exclude timestamp of log redirection in redirected logs. */
skipTime: true,
/** Level of log redirection */
level: LogLevel.DEBUG,
};

/**
* Logger for redirected actor logs.
*/
export class LoggerActorRedirect extends Logger {
constructor(options = {}) {
super({ ...DEFAULT_OPTIONS, ...options });
}

_console_log(line: string) {
console.log(line); // eslint-disable-line no-console
}

override _log(level: LogLevel, message: string, data?: any, exception?: unknown, opts: Record<string, any> = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering, what's the biggest difference between this implementation and the _log from LoggerText (link)? Could we subclass LoggerText instead of abstract Logger and reduce the amount of code here?

Copy link
Author

@Pijukatel Pijukatel Nov 3, 2025

Choose a reason for hiding this comment

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

Logger in our codebase is kind of a combination of a formater and a handler in Python terms. While LoggerActorRedirect has the same handler as LoggerText (they are both logging to console), they have completely different formater. So there is no point in inheriting from LoggerText as none of its methods would be reused in LoggerActorRedirect.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we need to adjust the shared logger to support this use case?

Copy link
Author

Choose a reason for hiding this comment

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

It is not really needed for this change. This change works with the current version of the codebase. Changing Log and Logger from the upstream repo to be more similar to the Python counterpart would be quite a refactoring.

if (level > this.options.level) {
return;
}
if (data || exception) {
throw new Error('Redirect logger does not use other arguments than level and message');
}
let { prefix } = opts;
prefix = prefix ? `${prefix}` : '';

let maybeDate = '';
if (!this.options.skipTime) {
maybeDate = `${new Date().toISOString().replace('Z', '').replace('T', ' ')} `;
Copy link
Member

Choose a reason for hiding this comment

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

do we need to repeat the timestamps?

Copy link
Author

@Pijukatel Pijukatel Nov 3, 2025

Choose a reason for hiding this comment

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

Those timestamps will not be the same. One is the timestamp of origin, and the other is the timestamp of redirection.

For example>
2025-10-31T09:04:03.380Z redirect-log-tester runId:V4OVPjPbmjflPDeMP -> 2025-10-31T09:03:26.899Z ACTOR: Pulling container image of build KNlNPqgcBBivdtNCr from registry.

It is important to

  • keep logs formatted in the same way. (Otherwise, redirected logs would be offset by not having the timestamp) - which is used to detect the actual log lines separations.
  • to keep it clear what the time of log redirection is and what the time of the actual log origin is (for redirected logs from actors that were started before we started redirection and to which we just attached)

}

const line = `${c.gray(maybeDate)}${c.cyan(prefix)}${message || ''}`;
this._console_log(line);
return line;
}
}

/**
* Helper class for redirecting streamed Actor logs to another log.
*/
export class StreamedLog {
private toLog: Log;
private streamBuffer: Buffer[] = [];
private splitMarker = /(?:\n|^)(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z)/g;
private relevancyTimeLimit: Date | null;

private logClient: LogClient;
private streamingTask: Promise<void> | null = null;
private stopLogging = false;

constructor(logClient: LogClient, toLog: Log, fromStart = true) {
this.toLog = toLog;
this.logClient = logClient;
this.relevancyTimeLimit = fromStart ? null : new Date();
}

/**
* Start log redirection.
*/
public async start(): Promise<void> {
if (this.streamingTask) {
throw new Error('Streaming task already active');
}
this.stopLogging = false;
this.streamingTask = this._streamLog();
return this.streamingTask;
}

/**
* Stop log redirection.
*/
public async stop(): Promise<void> {
if (!this.streamingTask) {
throw new Error('Streaming task is not active');
}
this.stopLogging = true;
try {
await this.streamingTask;
} catch (err) {
if (!(err instanceof Error && err.name === 'AbortError')) {
throw err;
}
} finally {
this.streamingTask = null;
}
}

/**
* Get log stream from response and redirect it to another log.
*/
private async _streamLog(): Promise<void> {
const logStream = await this.logClient.stream({ raw: true });
if (!logStream) {
return;
}

for await (const chunk of logStream) {
// Keep processing the new data until stopped
this.streamBuffer.push(chunk);
if (this.splitMarker.test(chunk.toString())) {
this.logBufferContent(false);
}
if (this.stopLogging) {
break;
}
}

// Process the remaining buffer
this.logBufferContent(true);
}

/**
* Parse the buffer and log complete messages.
*/
private logBufferContent(includeLastPart = false): void {
const allParts = Buffer.concat(this.streamBuffer).toString().split(this.splitMarker).slice(1);
let messageMarkers;
let messageContents;
// Parse the buffer parts into complete messages
if (includeLastPart) {
// This is final call, so log everything. Do not keep anything in the buffer.
messageMarkers = allParts.filter((_, i) => i % 2 === 0);
messageContents = allParts.filter((_, i) => i % 2 !== 0);
this.streamBuffer = [];
} else {
messageMarkers = allParts.filter((_, i) => i % 2 === 0).slice(0, -1);
messageContents = allParts.filter((_, i) => i % 2 !== 0).slice(0, -1);

// The last two parts (marker and message) are possibly not complete and will be left in the buffer.
this.streamBuffer = [Buffer.from(allParts.slice(-2).join(''))];
}

messageMarkers.forEach((marker, index) => {
const decodedMarker = marker;
const decodedContent = messageContents[index];
if (this.relevancyTimeLimit) {
// Log only relevant messages. Ignore too old log messages.
const logTime = new Date(decodedMarker);
if (logTime < this.relevancyTimeLimit) {
return;
}
}
const message = decodedMarker + decodedContent;

// Log parsed message at guessed level.
this.logAtGuessedLevel(message);
});
}

/**
* Log messages at appropriate log level guessed from the message content.
*
* Original log level information does not have to be included in the message at all.
* This is methods just guesses, exotic formating or specific keywords can break the guessing logic.
*/
private logAtGuessedLevel(message: string) {
message = message.trim();

if (message.includes('ERROR')) this.toLog.error(message);
else if (message.includes('SOFT_FAIL')) this.toLog.softFail(message);
else if (message.includes('WARNING')) this.toLog.warning(message);
else if (message.includes('INFO')) this.toLog.info(message);
else if (message.includes('DEBUG')) this.toLog.debug(message);
else if (message.includes('PERF')) this.toLog.perf(message);
// Fallback in case original log message does not indicate known log level.
else this.toLog.info(message);
}
}
31 changes: 30 additions & 1 deletion src/resource_clients/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { AxiosRequestConfig } from 'axios';
import ow from 'ow';

import type { RUN_GENERAL_ACCESS } from '@apify/consts';
import { LEVELS, Log } from '@apify/log';

import type { ApiClientOptionsWithOptionalResourcePath } from '../base/api_client';
import { ResourceClient } from '../base/resource_client';
Expand All @@ -10,7 +11,7 @@ import { cast, parseDateFields, pluckData } from '../utils';
import type { ActorRun } from './actor';
import { DatasetClient } from './dataset';
import { KeyValueStoreClient } from './key_value_store';
import { LogClient } from './log';
import { LogClient, LoggerActorRedirect, StreamedLog } from './log';
import { RequestQueueClient } from './request_queue';

const RUN_CHARGE_IDEMPOTENCY_HEADER = 'idempotency-key';
Expand Down Expand Up @@ -266,6 +267,34 @@ export class RunClient extends ResourceClient {
}),
);
}

/**
* Get StreamedLog for convenient streaming of the run log and their redirection.
*/
async getStreamedLog(options: GetStreamedLogOptions = {}): Promise<StreamedLog> {
const { fromStart = true } = options;
let { toLog } = options;
if (!toLog) {
// Get actor name and run id
const runData = await this.get();
const runId = runData ? `${runData.id ?? ''}` : '';

const actorId = runData?.actId ?? '';
const actorData = (await this.apifyClient.actor(actorId).get()) || { name: '' };

const actorName = runData ? (actorData.name ?? '') : '';
const name = [actorName, `runId:${runId}`].filter(Boolean).join(' ');

toLog = new Log({ level: LEVELS.DEBUG, prefix: `${name} -> `, logger: new LoggerActorRedirect() });
}

return new StreamedLog(this.log(), toLog, fromStart);
}
}

export interface GetStreamedLogOptions {
toLog?: Log;
fromStart?: boolean;
}

export interface RunGetOptions {
Expand Down
Loading
Loading