Fix logging middleware body reading and register extra params globally (#53)

* Fix body reading in logging middleware for non-JSON responses

The logging middleware was calling response.json() and falling back to
response.text() on the same cloned response. This fails when json()
fails to parse (e.g., HTML error pages from Cloudflare) because json()
consumes the body stream even when parsing fails.

Fix: Read as text() first, then try JSON.parse() on the string. This
only consumes the body once and handles non-JSON responses correctly.

* Register extra params middleware in global registry

The extra params (--extra-query, --extra-body, --extra-headers) were being
parsed but never applied to requests. This fix registers the extra params
middleware in the global middleware registry during BaseCommand.init(),
so it applies to ALL HTTP clients (WebDAV, OCAPI, SLAS, ODS, MRT, Custom APIs).

* Fix extra params middleware for GET/HEAD requests

- Skip adding body to GET/HEAD requests which don't allow request bodies
- Clean up global middleware registry between tests to prevent leakage
- Fix query params to use modifiedRequest instead of original request

Author: clavery

packages/b2c-tooling-sdk/src/cli/base-command.ts

@@ -14,7 +14,7 @@ import type {
 } from './hooks.js';
 import {setLanguage} from '../i18n/index.js';
 import {configureLogger, getLogger, type LogLevel, type Logger} from '../logging/index.js';
-import type {ExtraParamsConfig} from '../clients/middleware.js';
+import {createExtraParamsMiddleware, type ExtraParamsConfig} from '../clients/middleware.js';
 import type {ConfigSource} from '../config/types.js';
 import {globalMiddlewareRegistry} from '../clients/middleware-registry.js';
 
@@ -122,6 +122,10 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
 
     this.configureLogging();
 
+    // Register extra params middleware (from --extra-query, --extra-body, --extra-headers flags)
+    // This must happen before any API clients are created
+    this.registerExtraParamsMiddleware();
+
     // Collect middleware from plugins before any API clients are created
     await this.collectPluginHttpMiddleware();
 
@@ -357,4 +361,20 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
 
     return config;
   }
+
+  /**
+   * Register extra params (query, body, headers) as global middleware.
+   * This applies to ALL HTTP clients created during command execution.
+   */
+  private registerExtraParamsMiddleware(): void {
+    const extraParams = this.getExtraParams();
+    if (!extraParams) return;
+
+    globalMiddlewareRegistry.register({
+      name: 'cli-extra-params',
+      getMiddleware() {
+        return createExtraParamsMiddleware(extraParams);
+      },
+    });
+  }
 }

packages/b2c-tooling-sdk/src/clients/middleware.ts

@@ -165,10 +165,14 @@ export function createLoggingMiddleware(config?: string | LoggingMiddlewareConfi
 
       const clonedResponse = response.clone();
       let responseBody: unknown;
+      // Read as text first, then try to parse as JSON.
+      // This avoids a bug where json() consumes the body stream even when parsing fails,
+      // making subsequent text() calls fail with "Body has already been read".
+      const text = await clonedResponse.text();
       try {
-        responseBody = await clonedResponse.json();
+        responseBody = JSON.parse(text);
       } catch {
-        responseBody = await clonedResponse.text();
+        responseBody = text;
       }
 
       // Mask sensitive/large body keys before logging
@@ -208,6 +212,10 @@ export function createExtraParamsMiddleware(config: ExtraParamsConfig): Middlewa
     async onRequest({request}) {
       let modifiedRequest = request;
 
+      // HTTP methods that don't allow a request body
+      const methodsWithoutBody = ['GET', 'HEAD'];
+      const canHaveBody = !methodsWithoutBody.includes(modifiedRequest.method.toUpperCase());
+
       // Add extra headers first (before other modifications)
       if (config.headers && Object.keys(config.headers).length > 0) {
         const newHeaders = new Headers(modifiedRequest.headers);
@@ -218,28 +226,26 @@ export function createExtraParamsMiddleware(config: ExtraParamsConfig): Middlewa
         modifiedRequest = new Request(modifiedRequest.url, {
           method: modifiedRequest.method,
           headers: newHeaders,
-          body: modifiedRequest.body,
-          duplex: modifiedRequest.body ? 'half' : undefined,
+          ...(canHaveBody && modifiedRequest.body ? {body: modifiedRequest.body, duplex: 'half'} : {}),
         } as RequestInit);
       }
 
       // Add extra query parameters
       if (config.query && Object.keys(config.query).length > 0) {
-        const url = new URL(request.url);
+        const url = new URL(modifiedRequest.url);
         for (const [key, value] of Object.entries(config.query)) {
           if (value !== undefined) {
             url.searchParams.set(key, String(value));
           }
         }
         logger.trace(
-          {extraQuery: config.query, originalUrl: request.url, newUrl: url.toString()},
+          {extraQuery: config.query, originalUrl: modifiedRequest.url, newUrl: url.toString()},
           '[ExtraParams] Adding extra query params to URL',
         );
         modifiedRequest = new Request(url.toString(), {
-          method: request.method,
-          headers: request.headers,
-          body: request.body,
-          duplex: request.body ? 'half' : undefined,
+          method: modifiedRequest.method,
+          headers: modifiedRequest.headers,
+          ...(canHaveBody && modifiedRequest.body ? {body: modifiedRequest.body, duplex: 'half'} : {}),
         } as RequestInit);
       }
 
@@ -264,8 +270,8 @@ export function createExtraParamsMiddleware(config: ExtraParamsConfig): Middlewa
           } catch {
             logger.warn('[ExtraParams] Could not parse request body as JSON, skipping body merge');
           }
-        } else if (!modifiedRequest.body) {
-          // No existing body, create one with extra fields
+        } else if (!modifiedRequest.body && canHaveBody) {
+          // No existing body, create one with extra fields (only for methods that allow a body)
           logger.trace({body: config.body}, '[ExtraParams] Creating new body with extra fields');
           const headers = new Headers(modifiedRequest.headers);
           headers.set('content-type', 'application/json');

packages/b2c-tooling-sdk/test/cli/base-command.test.ts

@@ -6,6 +6,7 @@
 import {expect} from 'chai';
 import {Config} from '@oclif/core';
 import {BaseCommand} from '@salesforce/b2c-tooling-sdk/cli';
+import {globalMiddlewareRegistry} from '@salesforce/b2c-tooling-sdk/clients';
 
 // Create a concrete test command class
 class TestBaseCommand extends BaseCommand<typeof TestBaseCommand> {
@@ -57,6 +58,11 @@ describe('cli/base-command', () => {
     command = new TestBaseCommand([], config);
   });
 
+  afterEach(() => {
+    // Clean up the global middleware registry between tests
+    globalMiddlewareRegistry.clear();
+  });
+
   describe('init', () => {
     it('initializes command with default flags', async () => {
       // Mock parse method
@@ -401,23 +407,16 @@ describe('cli/base-command', () => {
         metadata: {},
       })) as typeof cmd.parse;
 
-      await cmd.init();
-      let errorCalled = false;
-      const originalError = cmd.error.bind(command);
-      cmd.error = () => {
-        errorCalled = true;
-        throw new Error('Expected error');
-      };
-
+      // Error is thrown during init() when registerExtraParamsMiddleware() calls getExtraParams()
+      let errorThrown = false;
       try {
-        command.testGetExtraParams();
-      } catch {
-        // Expected
+        await cmd.init();
+      } catch (err) {
+        errorThrown = true;
+        expect((err as Error).message).to.include('Invalid JSON for --extra-query');
       }
 
-      expect(errorCalled).to.be.true;
-
-      cmd.error = originalError;
+      expect(errorThrown).to.be.true;
       cmd.parse = originalParse;
     });
 
@@ -430,23 +429,16 @@ describe('cli/base-command', () => {
         metadata: {},
       })) as typeof cmd.parse;
 
-      await cmd.init();
-      let errorCalled = false;
-      const originalError = cmd.error.bind(command);
-      cmd.error = () => {
-        errorCalled = true;
-        throw new Error('Expected error');
-      };
-
+      // Error is thrown during init() when registerExtraParamsMiddleware() calls getExtraParams()
+      let errorThrown = false;
       try {
-        command.testGetExtraParams();
-      } catch {
-        // Expected
+        await cmd.init();
+      } catch (err) {
+        errorThrown = true;
+        expect((err as Error).message).to.include('Invalid JSON for --extra-body');
       }
 
-      expect(errorCalled).to.be.true;
-
-      cmd.error = originalError;
+      expect(errorThrown).to.be.true;
       cmd.parse = originalParse;
     });
 
@@ -497,23 +489,16 @@ describe('cli/base-command', () => {
         metadata: {},
       })) as typeof cmd.parse;
 
-      await cmd.init();
-      let errorCalled = false;
-      const originalError = cmd.error.bind(command);
-      cmd.error = () => {
-        errorCalled = true;
-        throw new Error('Expected error');
-      };
-
+      // Error is thrown during init() when registerExtraParamsMiddleware() calls getExtraParams()
+      let errorThrown = false;
       try {
-        command.testGetExtraParams();
-      } catch {
-        // Expected
+        await cmd.init();
+      } catch (err) {
+        errorThrown = true;
+        expect((err as Error).message).to.include('Invalid JSON for --extra-headers');
       }
 
-      expect(errorCalled).to.be.true;
-
-      cmd.error = originalError;
+      expect(errorThrown).to.be.true;
       cmd.parse = originalParse;
     });
   });