Review comments

Author: Pijukatel

src/resource_clients/actor.ts

@@ -7,14 +7,13 @@ import { Log } from '@apify/log';
 
 import type { ApiClientSubResourceOptions } from '../base/api_client';
 import { ResourceClient } from '../base/resource_client';
-import { cast, parseDateFields, pluckData, stringifyWebhooksToBase64 } from '../utils';
+import { cast, parseDateFields, pluckData, stringifyWebhooksToBase64 } from "../utils";
 import type { ActorVersion } from './actor_version';
 import { ActorVersionClient } from './actor_version';
 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';
@@ -154,24 +153,14 @@ export class ActorClient extends ResourceClient {
         // Creating a new instance of RunClient here would only allow
         // setting it up as a nested route under actor API.
         const newRunClient = this.apifyClient.run(id);
-        let streamedLog: StreamedLog;
-
-        // Log redirections is not compatible with browser environment
-        const isBrowser = typeof window !== 'undefined' && typeof window.document !== 'undefined';
-        if (options.log === null || isBrowser) {
-            return newRunClient.waitForFinish({ waitSecs });
-        }
-        if (options.log === undefined) {
-            streamedLog = await newRunClient.getStreamedLog();
-        } else {
-            streamedLog = await newRunClient.getStreamedLog({ toLog: options.log });
-        }
 
+        const streamedLog = await newRunClient.getStreamedLog({ toLog: options?.log });
         try {
-            await streamedLog.start();
+            await streamedLog?.start();
             return this.apifyClient.run(id).waitForFinish({ waitSecs });
-        } finally {
-            await streamedLog.stop();
+        }
+        finally {
+            await streamedLog?.stop();
         }
     }
 

src/resource_clients/run.ts

@@ -7,7 +7,7 @@ import { LEVELS, Log } from '@apify/log';
 import type { ApiClientOptionsWithOptionalResourcePath } from '../base/api_client';
 import { ResourceClient } from '../base/resource_client';
 import type { ApifyResponse } from '../http_client';
-import { cast, parseDateFields, pluckData } from '../utils';
+import { cast, isNode, parseDateFields, pluckData } from "../utils";
 import type { ActorRun } from './actor';
 import { DatasetClient } from './dataset';
 import { KeyValueStoreClient } from './key_value_store';
@@ -271,10 +271,15 @@ export class RunClient extends ResourceClient {
     /**
      * Get StreamedLog for convenient streaming of the run log and their redirection.
      */
-    async getStreamedLog(options: GetStreamedLogOptions = {}): Promise<StreamedLog> {
+    async getStreamedLog(options: GetStreamedLogOptions = {}): Promise<StreamedLog | undefined> {
         const { fromStart = true } = options;
         let { toLog } = options;
-        if (!toLog) {
+        if (toLog === null || !isNode()) {
+            // Explicitly no logging or not in Node.js
+            return undefined
+        }
+        if (toLog === undefined) {
+            // Create default StreamedLog
             // Get actor name and run id
             const runData = await this.get();
             const runId = runData ? `${runData.id ?? ''}` : '';
@@ -293,7 +298,7 @@ export class RunClient extends ResourceClient {
 }
 
 export interface GetStreamedLogOptions {
-    toLog?: Log;
+    toLog?: Log | null;
     fromStart?: boolean;
 }
 

test/mock_server/server.js

@@ -58,6 +58,21 @@ const mockServer = {
     },
 };
 
+async function streamLogChunks(req, res) {
+    // Asynchronously write each chunk to the response stream
+    for (const chunk of MOCKED_ACTOR_LOGS) {
+        res.write(chunk);
+        res.flush(); // Flush the buffer and send the chunk immediately
+        // Wait for a short period to simulate work being done on the server
+        await new Promise((resolve) => {
+            setTimeout(resolve, 10);
+        });
+    }
+
+    // End the response stream once all chunks have been sent
+    res.end();
+}
+
 // Debugging middleware
 app.use((req, res, next) => {
     next();
@@ -83,40 +98,14 @@ v2Router.use('/acts/redirect-actor-id', async (req, res) => {
 });
 v2Router.use('/acts', actorRouter);
 v2Router.use('/actor-builds', buildRouter);
-v2Router.use('/actor-runs/redirect-run-id/log', async (req, res) => {
-    // Asynchronously write each chunk to the response stream
-    for (const chunk of MOCKED_ACTOR_LOGS) {
-        res.write(chunk);
-        res.flush(); // Flush the buffer and send the chunk immediately
-        // Wait for a short period to simulate work being done on the server
-        await new Promise((resolve) => {
-            setTimeout(resolve, 10);
-        });
-    }
-
-    // End the response stream once all chunks have been sent
-    res.end();
-});
+v2Router.use('/actor-runs/redirect-run-id/log', streamLogChunks);
 v2Router.use('/actor-runs/redirect-run-id', async (req, res) => {
     res.json({ data: { id: 'redirect-run-id', actId: 'redirect-actor-id', status: 'SUCCEEDED' } });
 });
 v2Router.use('/actor-runs', runRouter);
 v2Router.use('/actor-tasks', taskRouter);
 v2Router.use('/users', userRouter);
-v2Router.use('/logs/redirect-log-id', async (req, res) => {
-    // Asynchronously write each chunk to the response stream
-    for (const chunk of MOCKED_ACTOR_LOGS) {
-        res.write(chunk);
-        res.flush(); // Flush the buffer and send the chunk immediately
-        // Wait for a short period to simulate work being done on the server
-        await new Promise((resolve) => {
-            setTimeout(resolve, 10);
-        });
-    }
-
-    // End the response stream once all chunks have been sent
-    res.end();
-});
+v2Router.use('/logs/redirect-log-id', streamLogChunks);
 v2Router.use('/logs', logRouter);
 v2Router.use('/datasets', datasetRouter);
 v2Router.use('/key-value-stores', keyValueStores);