Stop publishing the codegen node_modules

.github/workflows/release.yml

@@ -66,6 +66,9 @@ jobs:
       - name: Pack tarball
         run: npm pack --ignore-scripts
 
+      - name: Check tarball
+        run: bash tests/packaging/check-package.sh *.tgz
+
       - name: Attest build provenance
         uses: actions/attest-build-provenance@v4
         with:

.github/workflows/test.yml

@@ -27,6 +27,9 @@ jobs:
       - name: Lint
         run: yarn lint
 
+      - name: Check npm package
+        run: yarn check:package
+
   test-mongodb-backend:
     name: Test with MongoDB backend
     runs-on: ubuntu-24.04

README.md

@@ -14,6 +14,11 @@ The client generation is performed this way :
 yarn build
 ```
 
+The generated clients are shipped inside this package and load their dependencies from the root
+`package.json`, so any runtime dependency of a generated client must be declared there as well
+(`yarn build` fails otherwise).
+`yarn check:package` packs the build and verifies it installs and loads in a clean consumer.
+
 ### Local testing
 
 1. Install dependencies & build the smithy client: `yarn build`

eslint.config.mjs

@@ -34,5 +34,13 @@ export default tseslint.config(
             }],
             '@typescript-eslint/ban-ts-comment': 'off',
         }
-    }
+    },
+    {
+        // Runs as a plain CommonJS consumer of the published package
+        files: ['tests/packaging/**/*.js'],
+        rules: {
+            '@typescript-eslint/no-require-imports': 'off',
+            'no-console': 'off',
+        },
+    },
 );

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@scality/cloudserverclient",
-  "version": "1.0.12",
+  "version": "1.0.13",
   "engines": {
     "node": ">=20"
   },
@@ -19,27 +19,29 @@
   },
   "files": [
     "dist",
-    "build/smithy/cloudserverBackbeatRoutes/typescript-codegen",
-    "build/smithy/cloudserverBucketQuota/typescript-codegen",
-    "build/smithy/cloudserverProxyBackbeatApis/typescript-codegen"
+    "build/smithy/*/typescript-codegen/package.json",
+    "build/smithy/*/typescript-codegen/dist-*/**"
   ],
   "publishConfig": {
     "access": "public"
   },
   "scripts": {
     "clean:build": "rm -rf build dist",
-    "build:smithy": "smithy build && yarn clean:codegen-gitignore",
+    "build:smithy": "smithy build && yarn clean:codegen-gitignore && yarn check:deps",
+    "check:deps": "bash tests/packaging/check-deps.sh",
     "clean:codegen-gitignore": "rm -f build/smithy/*/typescript-codegen/.gitignore",
+    "clean:codegen-node-modules": "rm -rf build/smithy/*/typescript-codegen/node_modules",
     "build:generated:backbeatRoutes": "cd build/smithy/cloudserverBackbeatRoutes/typescript-codegen && yarn install && yarn build",
     "build:generated:bucketQuota": "cd build/smithy/cloudserverBucketQuota/typescript-codegen && yarn install && yarn build",
     "build:generated:proxyBackbeatApis": "cd build/smithy/cloudserverProxyBackbeatApis/typescript-codegen && yarn install && yarn build",
     "build:wrapper": "tsc",
-    "build": "yarn install && yarn clean:build && yarn build:smithy && yarn build:generated:backbeatRoutes && yarn build:generated:bucketQuota && yarn build:generated:proxyBackbeatApis && yarn build:wrapper",
+    "build": "yarn install && yarn clean:build && yarn build:smithy && yarn build:generated:backbeatRoutes && yarn build:generated:bucketQuota && yarn build:generated:proxyBackbeatApis && yarn clean:codegen-node-modules && yarn build:wrapper",
     "test": "NODE_OPTIONS='--experimental-vm-modules' jest",
     "test:mongo-backend": "BACKEND_TYPE=mongo yarn test",
     "test:metadata-backend": "BACKEND_TYPE=metadata yarn test",
     "test:backbeat-apis": "BACKBEAT_SETUP=true yarn test -- tests/testBackbeatProxyApis.test.ts",
     "lint": "eslint src tests",
+    "check:package": "bash tests/packaging/check-package.sh",
     "typecheck": "tsc --noEmit"
   },
   "devDependencies": {
@@ -56,9 +58,17 @@
   },
   "dependencies": {
     "@aws-sdk/client-s3": "^3.1009.0",
+    "@aws-sdk/core": "^3.978.1",
+    "@aws-sdk/credential-provider-node": "^3.972.84",
     "@aws-sdk/middleware-expect-continue": "^3.972.8",
+    "@aws-sdk/types": "^3.974.6",
+    "@smithy/core": "^3.35.0",
+    "@smithy/fetch-http-handler": "^5.8.0",
+    "@smithy/node-http-handler": "^4.12.1",
+    "@smithy/types": "^4.19.0",
     "JSONStream": "^1.3.5",
-    "fast-xml-parser": "^5.5.7"
+    "fast-xml-parser": "^5.5.7",
+    "tslib": "^2.6.2"
   },
   "resolutions": {
     "flatted": "^3.4.2"

smithy-build.json

@@ -3,8 +3,8 @@
     "sources": ["models", "service"],
     "maven": {
         "dependencies": [
-            "software.amazon.smithy:smithy-aws-traits:1.61.0",
-            "software.amazon.smithy.typescript:smithy-aws-typescript-codegen:0.34.0"
+            "software.amazon.smithy:smithy-aws-traits:1.73.0",
+            "software.amazon.smithy.typescript:smithy-aws-typescript-codegen:0.54.0"
         ]
     },
     "projections": {

src/clients/backbeatRoutes.ts

@@ -1,21 +1,16 @@
-import { 
-    CloudserverBackbeatRoutesClient, 
+import {
+    CloudserverBackbeatRoutesClient,
     CloudserverBackbeatRoutesClientConfig,
 } from '../../build/smithy/cloudserverBackbeatRoutes/typescript-codegen';
-import { createCustomErrorMiddleware } from '../utils';
+import { CloudserverRestJsonProtocol } from '../utils';
 
 export * from '../../build/smithy/cloudserverBackbeatRoutes/typescript-codegen';
 export class BackbeatRoutesClient extends CloudserverBackbeatRoutesClient {
     constructor(config: CloudserverBackbeatRoutesClientConfig) {
         super({
+            protocol: CloudserverRestJsonProtocol,
             ...config,
             signingEscapePath: false,
         });
-
-        this.middlewareStack.add(createCustomErrorMiddleware(), {
-            step: 'deserialize',
-            name: 'cloudserverErrorHandler',
-            priority: 'normal',
-        });
     }
 }

src/utils.ts

@@ -1,9 +1,16 @@
+import { AwsRestJsonProtocol } from '@aws-sdk/core/protocols';
 import { addExpectContinueMiddleware } from '@aws-sdk/middleware-expect-continue';
-import { MiddlewareStack, RequestHandler } from '@smithy/types';
-import { XMLParser } from 'fast-xml-parser';
+import { collectBody } from '@smithy/core/protocols';
 import {
-    CloudserverBackbeatRoutesServiceException
-} from '../build/smithy/cloudserverBackbeatRoutes/typescript-codegen';
+    HandlerExecutionContext,
+    HttpResponse,
+    MetadataBearer,
+    MiddlewareStack,
+    OperationSchema,
+    RequestHandler,
+    SerdeFunctions,
+} from '@smithy/types';
+import { XMLParser } from 'fast-xml-parser';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export type WithMiddlewareStack = { middlewareStack?: MiddlewareStack<any, any> };
@@ -63,14 +70,14 @@
         return;
     }
 
     const commandWithMiddleware = command as any;
     if (!commandWithMiddleware.middlewareStack) {
         throw new Error('Command does not have a middleware stack');
     }
 
     commandWithMiddleware.middlewareStack.add(
         (next: any) => async (args: any) => {
             const request = args.request as any;
             if (request?.headers && !request.headers['content-length']) {
                 request.headers['content-length'] = String(contentLength);
             }
@@ -82,72 +89,59 @@
     return;
 }
 
-export function createCustomErrorMiddleware() {
-    return (next: any) => async (args: any) => {
-        try {
-            return await next(args);
-        } catch (error: any) {
-            const parseXmlError = (xml: string) => {
-                try {
-                    const result = new XMLParser({}).parse(xml);
-                    return {
-                        code: result.Error?.Code,
-                        message: result.Error?.Message,
-                        requestId: result.Error?.RequestId,
-                    };
-                } catch (_parseError) {
-                    return {
-                        code: null,
-                        message: 'Malformed XML error response',
-                        requestId: null,
-                    };
-                }
-            };
-
-            const response = error.$response;
-            const statusCode = error.$metadata?.httpStatusCode;
-            const headers = response?.headers || {};
-            const contentType = (headers['content-type'] || '').toLowerCase();
-            if (contentType.includes('application/xml') || contentType.includes('text/xml')) {
-                const body = response?.body;
-                const xml = body?.toString() || '';
-                const errorInfo = parseXmlError(xml);
-
-                const xmlError: any = new CloudserverBackbeatRoutesServiceException({
-                    name: errorInfo.code || error.name,
-                    message: errorInfo.message || 'XML error response',
-                    $fault: statusCode >= 500 ? 'server' : 'client',
-                    $metadata: error.$metadata || {},
-                    $response: error.$response,
-                });
-                xmlError.parsedXml = errorInfo;
-                xmlError.code = errorInfo.code;
-
-                throw xmlError;
-            }
+function parseXmlError(body: string) {
+    try {
+        const { Error: error } = new XMLParser({}).parse(body);
+        return {
+            code: error?.Code || 'UnknownError',
+            message: error?.Message || 'XML error response',
+        };
+    } catch (_parseError) {
+        return {
+            code: 'UnknownError',
+            message: 'Malformed XML error response',
+        };
+    }
+}
 
-            const s3cNginxProxyResponse = contentType.includes('text/html');
-            if (s3cNginxProxyResponse) {
-                const body = response?.body;
-                const html = body?.toString() || '';
-                const title = html.match(/<title[^>]*>([^<]+)<\/title>/i);
-                const message = title && title[1] || 'HTML error response';
+function parseHtmlError(body: string, response: HttpResponse) {
+    const title = body.match(/<title[^>]*>([^<]+)<\/title>/i);
+    return {
+        code: `HTML ${response.reason || 'Error'}`,
+        message: title?.[1] || 'HTML error response',
+        rawBody: body,
+    };
+}
 
-                const htmlError: any = new CloudserverBackbeatRoutesServiceException({
-                    name: `HTML ${response?.reason || 'Error'}`,
-                    message,
-                    $fault: statusCode >= 500 ? 'server' : 'client',
-                    $metadata: error.$metadata || {},
-                    $response: error.$response,
-                });
-                htmlError.rawBody = html;
-
-                throw htmlError;
+/**
+ * restJson1 protocol that also understands the XML and HTML error responses
+ * that cloudserver and the S3C nginx proxy may return regardless of the
+ * service protocol. These bodies would otherwise fail JSON parsing: they are
+ * parsed here and handed to the regular error handling, so they map to the
+ * modeled error classes or to the service base exception, like JSON errors.
+ */
+export class CloudserverRestJsonProtocol extends AwsRestJsonProtocol {
+    async deserializeResponse<Output extends MetadataBearer>(
+        operationSchema: OperationSchema,
+        context: HandlerExecutionContext & SerdeFunctions,
+        response: HttpResponse,
+    ): Promise<Output> {
+        if (response.statusCode >= 300) {
+            const contentType = String(response.headers['content-type'] || '').toLowerCase();
+            const isXml = contentType.includes('application/xml') || contentType.includes('text/xml');
+            const isHtml = contentType.includes('text/html');
+            if (isXml || isHtml) {
+                const body = (await collectBody(response.body, context)).transformToString();
+                // Keep the raw body readable on the error's $response
+                // eslint-disable-next-line no-param-reassign
+                response.body = body;
+                const errorData = isXml ? parseXmlError(body) : parseHtmlError(body, response);
+                return this.handleError(operationSchema, context, response, errorData,
+                    this.deserializeMetadata(response));
             }
-
-            throw error;
         }
-    };
+        return super.deserializeResponse(operationSchema, context, response);
+    }
 }
 
 export function attachReqUids(s3req, uuid: string) {

tests/packaging/check-deps.sh

@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+# The generated clients are shipped inside this package, so their runtime
+# dependencies must be declared in the root package.json too.
+set -euo pipefail
+
+cd "$(dirname "$0")/../.."
+
+generated=(build/smithy/*/typescript-codegen/package.json)
+
+missing=$(comm -23 \
+    <(jq -r '.dependencies // {} | keys[]' "${generated[@]}" | sort -u) \
+    <(jq -r '.dependencies // {} | keys[]' package.json | sort -u))
+
+if [ -n "$missing" ]; then
+    echo "ERROR: dependencies of the generated clients missing from package.json, add them with:" >&2
+    echo "  yarn add $(for name in $missing; do
+        jq -r --arg n "$name" '.dependencies[$n] // empty | "\($n)@\(.)"' "${generated[@]}" | head -1
+    done | tr '\n' ' ')" >&2
+    exit 1
+fi

tests/packaging/check-package.sh

@@ -0,0 +1,32 @@
+#!/bin/bash
+# Installs the npm tarball into a scratch consumer and checks it loads, sends
+# requests and type-checks there.
+#
+# Usage: check-package.sh [package.tgz]   (packs the current build if omitted)
+set -euo pipefail
+
+HERE=$(cd "$(dirname "$0")" && pwd)
+WORK_DIR=$(mktemp -d)
+trap 'rm -rf "$WORK_DIR"' EXIT
+
+if [ $# -ge 1 ]; then
+    TARBALL=$(cd "$(dirname "$1")" && pwd)/$(basename "$1")
+else
+    (cd "$HERE/../.." && npm pack --ignore-scripts --silent --pack-destination "$WORK_DIR" >/dev/null)
+    TARBALL=$(ls "$WORK_DIR"/*.tgz)
+fi
+
+if tar -tzf "$TARBALL" | grep -q '/node_modules/'; then
+    echo "ERROR: $(basename "$TARBALL") contains node_modules" >&2
+    exit 1
+fi
+
+cd "$WORK_DIR"
+cp "$HERE/consumer.package.json" package.json
+cp "$HERE/smoke.js" "$HERE/types.ts" "$HERE/tsconfig.json" .
+npm install --omit=dev --ignore-scripts --no-audit --no-fund --silent "$TARBALL"
+node smoke.js
+
+npm install --no-save --ignore-scripts --no-audit --no-fund --silent typescript @types/node@20
+npx --no-install tsc -p tsconfig.json
+echo "types OK"

tests/packaging/consumer.package.json

@@ -0,0 +1,5 @@
+{
+  "name": "cloudserverclient-packaging-consumer",
+  "version": "0.0.0",
+  "private": true
+}

tests/packaging/smoke.js

@@ -0,0 +1,37 @@
+// Sends one request with each generated client, from a consumer that only
+// has the published tarball installed.
+const assert = require('assert');
+const http = require('http');
+const pkg = require('@scality/cloudserverclient');
+require('@scality/cloudserverclient/clients/backbeatRoutes');
+require('@scality/cloudserverclient/clients/bucketQuota');
+require('@scality/cloudserverclient/clients/proxyBackbeatApis');
+
+const requests = [];
+const server = http.createServer((req, res) => {
+    requests.push(`${req.method} ${req.url}`);
+    assert.match(req.headers.authorization || '', /^AWS4-HMAC-SHA256 /);
+    req.resume();
+    req.on('end', () => res.writeHead(200, { 'content-length': 0 }).end());
+});
+
+server.listen(0, '127.0.0.1', async () => {
+    const client = new pkg.CloudserverClient({
+        endpoint: `http://127.0.0.1:${server.address().port}`,
+        region: 'us-east-1',
+        credentials: { accessKeyId: 'accessKey', secretAccessKey: 'secretKey' },
+        maxAttempts: 1,
+    });
+    try {
+        await client.backbeatRoutes.send(new pkg.GetRaftIdCommand({ Bucket: 'bucket' }));
+        await client.bucketQuota.send(new pkg.GetBucketQuotaCommand({ Bucket: 'bucket' }));
+        await client.proxyBackbeatApis.send(new pkg.CheckConnectionCommand({}));
+        assert.strictEqual(requests.length, 3);
+        console.log(`runtime OK: ${requests.join(', ')}`);
+    } catch (err) {
+        console.error(err);
+        process.exitCode = 1;
+    } finally {
+        server.close();
+    }
+});

tests/packaging/tsconfig.json

@@ -0,0 +1,12 @@
+{
+    "compilerOptions": {
+        "module": "node16",
+        "moduleResolution": "node16",
+        "target": "es2022",
+        "strict": true,
+        "noEmit": true,
+        "skipLibCheck": false,
+        "types": ["node"]
+    },
+    "files": ["types.ts"]
+}