feat: Refactor(logging): centralize HTTP error logging (#336)

jirispilka · web-flow · commit 7b0a52d1e3ba · 2025-11-14T13:29:54.000+01:00
* feat: Refactor(logging): centralize HTTP error logging with logHttpError utility

* fix: Do not log stack-trace for soft-fail
diff --git a/src/actor/server.ts b/src/actor/server.ts
@@ -26,7 +26,14 @@ export function createExpressApp(
     const transports: { [sessionId: string]: StreamableHTTPServerTransport } = {};
 
     function respondWithError(res: Response, error: unknown, logMessage: string, statusCode = 500) {
-        log.error('Error in request', { logMessage, error });
+        if (statusCode >= 500) {
+            // Server errors (>= 500) - log as exception
+            log.exception(error instanceof Error ? error : new Error(String(error)), 'Error in request', { logMessage, statusCode });
+        } else {
+            // Client errors (< 500) - log as softFail without stack trace
+            const errorMessage = error instanceof Error ? error.message : String(error);
+            log.softFail('Error in request', { logMessage, error: errorMessage, statusCode });
+        }
         if (!res.headersSent) {
             res.status(statusCode).json({
                 jsonrpc: '2.0',
@@ -105,7 +112,7 @@ export function createExpressApp(
             });
             const sessionId = new URL(req.url, `http://${req.headers.host}`).searchParams.get('sessionId');
             if (!sessionId) {
-                log.error('No session ID provided in POST request');
+                log.softFail('No session ID provided in POST request', { statusCode: 400 });
                 res.status(400).json({
                     jsonrpc: '2.0',
                     error: {
@@ -120,7 +127,7 @@ export function createExpressApp(
             if (transport) {
                 await transport.handlePostMessage(req, res);
             } else {
-                log.error('Server is not connected to the client.');
+                log.softFail('Server is not connected to the client.', { statusCode: 400 });
                 res.status(400).json({
                     jsonrpc: '2.0',
                     error: {
@@ -217,7 +224,7 @@ export function createExpressApp(
             return;
         }
 
-        log.error('Session not found', { sessionId });
+        log.softFail('Session not found', { sessionId, statusCode: 400 });
         res.status(400).send('Bad Request: Session not found').end();
     });
 
diff --git a/src/mcp/client.ts b/src/mcp/client.ts
@@ -5,6 +5,7 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/
 import log from '@apify/log';
 
 import { TimeoutError } from '../errors.js';
+import { logHttpError } from '../utils/logging.js';
 import { ACTORIZED_MCP_CONNECTION_TIMEOUT_MSEC } from './const.js';
 import { getMCPServerID } from './utils.js';
 
@@ -40,8 +41,7 @@ export async function connectMCPClient(
             log.warning('Connection to MCP server using SSE transport timed out', { url });
             return null;
         }
-
-        log.error('Failed to connect to MCP server using SSE transport', { cause: error });
+        logHttpError(error, 'Failed to connect to MCP server using SSE transport', { url, cause: error });
         throw error;
     }
 }
diff --git a/src/mcp/server.ts b/src/mcp/server.ts
@@ -39,6 +39,7 @@ import { callActorGetDataset, defaultTools, getActorsAsTools, toolCategories } f
 import { decodeDotPropertyNames } from '../tools/utils.js';
 import type { ToolEntry } from '../types.js';
 import { buildActorResponseContent } from '../utils/actor-response.js';
+import { logHttpError } from '../utils/logging.js';
 import { buildMCPResponse } from '../utils/mcp.js';
 import { createProgressTracker } from '../utils/progress.js';
 import { cloneToolEntry, getToolPublicFieldOnly } from '../utils/tools.js';
@@ -483,7 +484,7 @@ export class ActorsMcpServer {
             // Validate token
             if (!apifyToken && !this.options.skyfireMode) {
                 const msg = 'APIFY_TOKEN is required. It must be set in the environment variables or passed as a parameter in the body.';
-                log.error(msg);
+                log.softFail(msg, { statusCode: 400 });
                 await this.server.sendLoggingMessage({ level: 'error', data: msg });
                 throw new McpError(
                     ErrorCode.InvalidParams,
@@ -507,7 +508,7 @@ export class ActorsMcpServer {
                 .find((t) => t.name === name || (t.type === 'actor' && t.actorFullName === name));
             if (!tool) {
                 const msg = `Tool ${name} not found. Available tools: ${this.listToolNames().join(', ')}`;
-                log.error(msg);
+                log.softFail(msg, { statusCode: 404 });
                 await this.server.sendLoggingMessage({ level: 'error', data: msg });
                 throw new McpError(
                     ErrorCode.InvalidParams,
@@ -516,7 +517,7 @@ export class ActorsMcpServer {
             }
             if (!args) {
                 const msg = `Missing arguments for tool ${name}`;
-                log.error(msg);
+                log.softFail(msg, { statusCode: 400 });
                 await this.server.sendLoggingMessage({ level: 'error', data: msg });
                 throw new McpError(
                     ErrorCode.InvalidParams,
@@ -529,7 +530,7 @@ export class ActorsMcpServer {
             log.debug('Validate arguments for tool', { toolName: tool.name, input: args });
             if (!tool.ajvValidate(args)) {
                 const msg = `Invalid arguments for tool ${tool.name}: args: ${JSON.stringify(args)} error: ${JSON.stringify(tool?.ajvValidate.errors)}`;
-                log.error(msg);
+                log.softFail(msg, { statusCode: 400 });
                 await this.server.sendLoggingMessage({ level: 'error', data: msg });
                 throw new McpError(
                     ErrorCode.InvalidParams,
@@ -569,7 +570,9 @@ export class ActorsMcpServer {
                         client = await connectMCPClient(tool.serverUrl, apifyToken);
                         if (!client) {
                             const msg = `Failed to connect to MCP server ${tool.serverUrl}`;
-                            log.error(msg);
+                            // Note: Timeout errors are already logged as warning in connectMCPClient
+                            // This is a fallback log for when connection fails (client-side issue)
+                            log.softFail(msg, { statusCode: 408 }); // 408 Request Timeout
                             await this.server.sendLoggingMessage({ level: 'error', data: msg });
                             return {
                                 content: [
@@ -663,15 +666,15 @@ export class ActorsMcpServer {
                     }
                 }
             } catch (error) {
-                log.error('Error occurred while calling tool', { toolName: name, error });
+                logHttpError(error, 'Error occurred while calling tool', { toolName: name });
                 const errorMessage = (error instanceof Error) ? error.message : 'Unknown error';
                 return buildMCPResponse([
                     `Error calling tool ${name}: ${errorMessage}`,
                 ]);
             }
 
             const msg = `Unknown tool: ${name}`;
-            log.error(msg);
+            log.softFail(msg, { statusCode: 404 });
             await this.server.sendLoggingMessage({
                 level: 'error',
                 data: msg,
diff --git a/src/tools/actor.ts b/src/tools/actor.ts
@@ -24,6 +24,7 @@ import { ensureOutputWithinCharLimit, getActorDefinitionStorageFieldNames, getAc
 import { fetchActorDetails } from '../utils/actor-details.js';
 import { buildActorResponseContent } from '../utils/actor-response.js';
 import { ajv } from '../utils/ajv.js';
+import { logHttpError } from '../utils/logging.js';
 import { buildMCPResponse } from '../utils/mcp.js';
 import type { ProgressTracker } from '../utils/progress.js';
 import type { JsonSchemaProperty } from '../utils/schema-generation.js';
@@ -84,7 +85,7 @@ export async function callActorGetDataset(
             try {
                 await apifyClient.run(actorRun.id).abort({ gracefully: false });
             } catch (e) {
-                log.error('Error aborting Actor run', { error: e, runId: actorRun.id });
+                logHttpError(e, 'Error aborting Actor run', { runId: actorRun.id });
             }
             // Reject to stop waiting
             resolve(CLIENT_ABORT);
@@ -245,11 +246,9 @@ async function getMCPServersAsTools(
             }
             return await getMCPServerTools(actorId, client, mcpServerUrl);
         } catch (error) {
-            // Server error - log and continue processing other actors
-            log.error('Failed to connect to MCP server', {
+            logHttpError(error, 'Failed to connect to MCP server', {
                 actorFullName: actorInfo.actorDefinitionPruned.actorFullName,
                 actorId,
-                error,
             });
             return [];
         } finally {
@@ -294,10 +293,8 @@ export async function getActorsAsTools(
                     webServerMcpPath: getActorMCPServerPath(actorDefinitionPruned),
                 } as ActorInfo;
             } catch (error) {
-                // Server error - log and continue processing other actors
-                log.error('Failed to fetch Actor definition', {
+                logHttpError(error, 'Failed to fetch Actor definition', {
                     actorName: actorIdOrName,
-                    error,
                 });
                 return null;
             }
@@ -542,7 +539,7 @@ EXAMPLES:
 
             return { content };
         } catch (error) {
-            log.error('Failed to call Actor', { error, actorName, performStep });
+            logHttpError(error, 'Failed to call Actor', { actorName, performStep });
             return buildMCPResponse([`Failed to call Actor '${actorName}': ${error instanceof Error ? error.message : String(error)}`]);
         }
     },
diff --git a/src/tools/fetch-apify-docs.ts b/src/tools/fetch-apify-docs.ts
@@ -1,13 +1,12 @@
 import { z } from 'zod';
 import zodToJsonSchema from 'zod-to-json-schema';
 
-import log from '@apify/log';
-
 import { HelperTools } from '../const.js';
 import { fetchApifyDocsCache } from '../state.js';
 import type { InternalToolArgs, ToolEntry, ToolInputSchema } from '../types.js';
 import { ajv } from '../utils/ajv.js';
 import { htmlToMarkdown } from '../utils/html-to-md.js';
+import { logHttpError } from '../utils/logging.js';
 
 const fetchApifyDocsToolArgsSchema = z.object({
     url: z.string()
@@ -53,6 +52,11 @@ USAGE EXAMPLES:
             try {
                 const response = await fetch(url);
                 if (!response.ok) {
+                    // Create error object with statusCode for logHttpError
+                    const error = Object.assign(new Error(`HTTP ${response.status} ${response.statusText}`), {
+                        statusCode: response.status,
+                    });
+                    logHttpError(error, 'Failed to fetch the documentation page', { url, statusText: response.statusText });
                     return {
                         content: [{
                             type: 'text',
@@ -66,7 +70,7 @@ USAGE EXAMPLES:
                 // Use the URL without fragment as the key to avoid caching same page with different fragments
                 fetchApifyDocsCache.set(urlWithoutFragment, markdown);
             } catch (error) {
-                log.error('Failed to fetch the documentation page', { url, error });
+                logHttpError(error, 'Failed to fetch the documentation page', { url });
                 return {
                     content: [{
                         type: 'text',
diff --git a/src/utils/actor-details.ts b/src/utils/actor-details.ts
@@ -1,11 +1,10 @@
 import type { Actor, Build } from 'apify-client';
 
-import log from '@apify/log';
-
 import type { ApifyClient } from '../apify-client.js';
 import { filterSchemaProperties, shortenProperties } from '../tools/utils.js';
 import type { IActorInputSchema } from '../types.js';
 import { formatActorToActorCard } from './actor-card.js';
+import { logHttpError } from './logging.js';
 
 // Keep the interface here since it is a self contained module
 export interface ActorDetailsResult {
@@ -38,19 +37,7 @@ export async function fetchActorDetails(apifyClient: ApifyClient, actorName: str
             readme: buildInfo.actorDefinition.readme || 'No README provided.',
         };
     } catch (error) {
-        // Check if it's a 404 error (actor not found) - this is expected
-        const is404 = typeof error === 'object'
-            && error !== null
-            && 'statusCode' in error
-            && (error as { statusCode?: number }).statusCode === 404;
-
-        if (is404) {
-            // Log 404 errors at info level since they're expected (user may query non-existent actors)
-            log.info(`Actor '${actorName}' not found`, { actorName });
-        } else {
-            // Log other errors at error level
-            log.error(`Failed to fetch actor details for '${actorName}'`, { actorName, error });
-        }
+        logHttpError(error, `Failed to fetch actor details for '${actorName}'`, { actorName });
         return null;
     }
 }
diff --git a/src/utils/logging.ts b/src/utils/logging.ts
@@ -0,0 +1,54 @@
+import log from '@apify/log';
+
+/**
+ * Safely extract HTTP status code from errors.
+ * Checks both `statusCode` and `code` properties for compatibility.
+ */
+export function getHttpStatusCode(error: unknown): number | undefined {
+    if (typeof error !== 'object' || error === null) {
+        return undefined;
+    }
+
+    // Check for statusCode property (used by apify-client)
+    if ('statusCode' in error) {
+        const { statusCode } = (error as { statusCode?: unknown });
+        if (typeof statusCode === 'number' && statusCode >= 100 && statusCode < 600) {
+            return statusCode;
+        }
+    }
+
+    // Check for code property (used by some error types)
+    if ('code' in error) {
+        const { code } = (error as { code?: unknown });
+        if (typeof code === 'number' && code >= 100 && code < 600) {
+            return code;
+        }
+    }
+
+    return undefined;
+}
+
+/**
+ * Logs HTTP errors based on status code, following apify-core pattern.
+ * Uses `softFail` for status < 500 (API client errors) and `exception` for status >= 500 (API server errors).
+ *
+ * @param error - The error object
+ * @param message - The log message
+ * @param data - Additional data to include in the log
+ */
+export function logHttpError<T extends object>(error: unknown, message: string, data?: T): void {
+    const statusCode = getHttpStatusCode(error);
+    const errorMessage = error instanceof Error ? error.message : String(error);
+
+    if (statusCode !== undefined && statusCode < 500) {
+        // Client errors (< 500) - log as softFail without stack trace
+        log.softFail(message, { error: errorMessage, statusCode, ...data });
+    } else if (statusCode !== undefined && statusCode >= 500) {
+        // Server errors (>= 500) - log as exception with full error (includes stack trace)
+        const errorObj = error instanceof Error ? error : new Error(String(error));
+        log.exception(errorObj, message, { statusCode, ...data });
+    } else {
+        // No status code available - log as error
+        log.error(message, { error, ...data });
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/`
`5`	`5`	`import log from '@apify/log';`
`6`	`6`
`7`	`7`	`import { TimeoutError } from '../errors.js';`
	`8`	`+import { logHttpError } from '../utils/logging.js';`
`8`	`9`	`import { ACTORIZED_MCP_CONNECTION_TIMEOUT_MSEC } from './const.js';`
`9`	`10`	`import { getMCPServerID } from './utils.js';`
`10`	`11`
`@@ -40,8 +41,7 @@ export async function connectMCPClient(`
`40`	`41`	`log.warning('Connection to MCP server using SSE transport timed out', { url });`
`41`	`42`	`return null;`
`42`	`43`	`}`
`43`		`-`
`44`		`- log.error('Failed to connect to MCP server using SSE transport', { cause: error });`
	`44`	`+ logHttpError(error, 'Failed to connect to MCP server using SSE transport', { url, cause: error });`
`45`	`45`	`throw error;`
`46`	`46`	`}`
`47`	`47`	`}`