Skip to content

Use JSON.stringify for object interpolation in log statements to avoi… #311

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
rhopp wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use JSON.stringify for object interpolation in log statements to avoi… #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2319f7b
Use JSON.stringify for object interpolation in log statements to avoi…
rhopp Feb 27, 2026
79c8d0d
Sanitize log output: avoid leaking sensitive data and fix error handl…
rhopp Feb 27, 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
57 changes: 44 additions & 13 deletions src/api/azure/http/azure-http.client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
import { BaseHttpClient } from '../../common/http/base-http.client';
import { AxiosError } from 'axios';
import { ApiError } from '../../common/errors/api.errors';
import { AzureApiError } from '../errors/azure.errors';
import { LoggerFactory, Logger } from '../../../logger/logger';

function sanitizeErrorData(data: unknown): string {
if (data == null) return 'no data';
if (typeof data === 'string') return data.slice(0, 2000);
if (typeof data === 'object') {
const { message, typeKey, errorCode, statusCode } = data as Record<string, unknown>;
const parts = [
message != null ? `message=${message}` : null,
typeKey != null ? `typeKey=${typeKey}` : null,
errorCode != null ? `errorCode=${errorCode}` : null,
statusCode != null ? `statusCode=${statusCode}` : null,
].filter(Boolean);
return parts.length > 0 ? parts.join(', ') : '[response data omitted]';
}
return String(data);
}

export interface AzureHttpClientConfig {
organization: string;
host: string;
Expand Down Expand Up @@ -43,21 +59,36 @@ export class AzureHttpClient extends BaseHttpClient {

this.client.interceptors.response.use(
response => response,
(error: AxiosError) => {
if (error.response) {
this.logger.error(`Azure DevOps API Error: ${error.response.status} ${error.response.statusText} - ${error.response.data}`);
(error) => {
// Build the AzureApiError first, so wrapping always succeeds regardless of logging
let azureError: AzureApiError;

// BaseHttpClient already transforms AxiosError → ApiError
// Map ApiError to Azure-specific errors first
if (error instanceof ApiError) {
azureError = new AzureApiError(error.message, error.status, error.data, error);
try {
this.logger.error(`Azure DevOps API Error: ${error.status} - ${sanitizeErrorData(error.data)}`);
} catch { /* logging must not break error propagation */ }
} else if (error.response) {
// Fallback: Handle raw AxiosError if BaseHttpClient interceptor didn't catch it
azureError = new AzureApiError(error.message, error.response.status, error.response.data, error);
try {
this.logger.error(`Azure DevOps API Error: ${error.response.status} ${error.response.statusText} - ${sanitizeErrorData(error.response.data)}`);
} catch { /* logging must not break error propagation */ }
} else if (error.request) {
this.logger.error(`Azure DevOps API Error: No response received - ${error.request}`);
azureError = new AzureApiError('No response received from Azure DevOps', undefined, undefined, error);
try {
const { method, baseURL, url, timeout } = error.request?.config ?? error.config ?? {};
this.logger.error(`Azure DevOps API Error: No response received - ${method?.toUpperCase()} ${baseURL ?? ''}${url ?? ''} (timeout: ${timeout})`);
Comment on lines +82 to +83

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n src/api/azure/http/azure-http.client.ts | sed -n '75,95p'

Repository: redhat-appstudio/tssc-test

Length of output: 1415

🏁 Script executed:

rg -n "error\.request" src/api/azure/http/azure-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 210

🏁 Script executed:

rg -n "error\.request\?*\.config|error\.request\.config" --type ts src/

Repository: redhat-appstudio/tssc-test

Length of output: 211

🏁 Script executed:

rg -n "import.*axios|import.*AxiosError" src/api/azure/http/azure-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 52

🏁 Script executed:

rg -n "AxiosError|error:" src/api/azure/http/azure-http.client.ts -B 2 -A 2

Repository: redhat-appstudio/tssc-test

Length of output: 626

🏁 Script executed:

# Check if error is explicitly typed and how it's imported
rg -n "catch\s*\(\s*error" src/api/azure/http/azure-http.client.ts -A 1

Repository: redhat-appstudio/tssc-test

Length of output: 52

🏁 Script executed:

# Look for axios client setup to understand error handling context
head -50 src/api/azure/http/azure-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 1754

🏁 Script executed:

# Check for AxiosError type definitions or error type annotations
rg -n "catch.*error.*AxiosError|error\s*:\s*AxiosError|as AxiosError" src/api/azure/http/azure-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 52

🏁 Script executed:

# Check BaseHttpClient to understand what error types propagate
head -100 src/api/common/http/base-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 3206

🏁 Script executed:

# Search for axios usage in BaseHttpClient
rg -n "error\.request\|\.request\?\." src/api/common/http/base-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 52

🏁 Script executed:

# Check ApiError class to understand error.cause
rg -n "class ApiError|error\.cause" src/api/common/errors/api.errors.ts -A 5

Repository: redhat-appstudio/tssc-test

Length of output: 435

🏁 Script executed:

# Look at the full error handling block to understand context
sed -n '60,95p' src/api/azure/http/azure-http.client.ts

Repository: redhat-appstudio/tssc-test

Length of output: 2065

🏁 Script executed:

# Check the full ApiError class structure
cat -n src/api/common/errors/api.errors.ts

Repository: redhat-appstudio/tssc-test

Length of output: 3862

🏁 Script executed:

# Verify that in the else if (error.request) block, error is indeed AxiosError
# by checking what properties are being accessed
rg -n "error\.request|error\.config|error\.response" src/api/azure/http/azure-http.client.ts | head -20

Repository: redhat-appstudio/tssc-test

Length of output: 520

Remove dead code path—error.request doesn't have a config property.

In AxiosError, error.request is the Node.js http.ClientRequest object, which has no config property. The expression error.request?.config will always evaluate to undefined, making it unnecessary dead code. Simplify to access only error.config:

Suggested fix 
-          const { method, baseURL, url, timeout } = error.request?.config ?? error.config ?? {};
+          const { method, baseURL, url, timeout } = error.config ?? {};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/api/azure/http/azure-http.client.ts` around lines 82 - 83, The
conditional uses a dead branch error.request?.config (error.request is an
http.ClientRequest and has no config); update the extraction to pull method,
baseURL, url, timeout only from error.config (e.g., replace the destructuring
that uses error.request?.config ?? error.config ?? {} with just error.config ??
{}), and keep the existing this.logger.error call (Azure DevOps API Error...)
unchanged so it logs method/baseURL/url/timeout from the valid AxiosError
config; target the destructuring near the this.logger.error call in
azure-http.client.ts.

} catch { /* logging must not break error propagation */ }
} else {
this.logger.error(`Azure DevOps API Error: Request setup failed - ${error}`);
azureError = new AzureApiError('Request setup failed', undefined, undefined, error);
try {
this.logger.error(`Azure DevOps API Error: Request setup failed - ${error}`);
} catch { /* logging must not break error propagation */ }
Comment on lines +86 to +89

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid raw object interpolation in setup-failure logging.

This branch can still emit [object Object] for non-Error values, which weakens debugging output.

Suggested fix 
         } else {
           azureError = new AzureApiError('Request setup failed', undefined, undefined, error);
           try {
-            this.logger.error(`Azure DevOps API Error: Request setup failed - ${error}`);
+            const detail = error instanceof Error ? error.message : sanitizeErrorData(error);
+            this.logger.error(`Azure DevOps API Error: Request setup failed - ${detail}`);
           } catch { /* logging must not break error propagation */ }
         }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/api/azure/http/azure-http.client.ts` around lines 86 - 89, The logging
currently interpolates the raw error (this.logger.error(`Azure DevOps API Error:
Request setup failed - ${error}`)) which can produce “[object Object]”; update
the catch branch in azure-http.client.ts to serialize the error safely before
logging: if the value is an Error log error.stack (or error.message if stack
missing); otherwise attempt JSON.stringify(error) with a try/catch fallback to
util.inspect(error) or String(error); keep creating azureError (AzureApiError)
as before but pass the serialized message to this.logger.error to ensure useful
output.

}
// Wrap AxiosError in a custom AzureApiError
const azureError = new AzureApiError(
error.message,
error.response?.status,
error.response?.data,
error
);

return Promise.reject(azureError);
}
);
Expand Down
3 changes: 2 additions & 1 deletion src/api/azure/services/azure-variable-group.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export class AzureVariableGroupService {
`${this.project}/_apis/distributedtask/variablegroups?${this.getApiVersionParam()}`,
payload
);
this.logger.info(`AzureCI group creation response: ${response}`);
const { id, name } = (response as any) ?? {};
this.logger.info(`Variable group created: id=${id}, name=${name}`);
} catch (error) {
this.logger.error(`Failed to create variable group '${groupName}': ${error}`);
throw error;
Expand Down
3 changes: 2 additions & 1 deletion src/api/rhdh/developerhub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export class DeveloperHub {
componentScaffoldOptions: ScaffolderScaffoldOptions
): Promise<ComponentIdResponse> {
try {
this.logger.info(`Creating component with options: ${componentScaffoldOptions}`);
const { templateRef, values: { name, repoName, namespace, ciType, hostType } = {} } = componentScaffoldOptions ?? {};
this.logger.info(`Creating component: template=${templateRef}, name=${name}, repo=${repoName}, namespace=${namespace}, ci=${ciType}, host=${hostType}`);
const response: AxiosResponse<ComponentIdResponse> = await this.axios.post(
`${this.url}/api/scaffolder/v2/tasks`,
componentScaffoldOptions
Expand Down