Merge branch 'main' into rlamb/SDK-1375/only-support-identify-with-result

Author: kinyoklion

.github/workflows/shopify-oxygen.yml

@@ -26,3 +26,15 @@ jobs:
         with:
           workspace_name: '@launchdarkly/shopify-oxygen-sdk'
           workspace_path: packages/sdk/shopify-oxygen
+      - name: Install contract test service dependencies
+        run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests install --no-immutable
+      - name: Build the test service
+        run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests build
+      - name: Launch the test service in the background
+        run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests start &> /dev/null &
+      - uses: launchdarkly/gh-actions/actions/contract-tests@21174f3a7f3aa3e3121227ec91842e8a1ebeec6e
+        with:
+          test_service_port: 8000
+          token: ${{ secrets.GITHUB_TOKEN }}
+          # Based on run-test-harness.sh from the sdk package
+          extra_params: '--url http://localhost:8000 --skip "streaming.*" --skip "evaluation.*" --skip "event.*" --skip "service.*" --stop-service-at-end'

package.json

@@ -42,7 +42,8 @@
     "packages/telemetry/browser-telemetry",
     "contract-tests",
     "packages/sdk/combined-browser",
-    "packages/sdk/shopify-oxygen"
+    "packages/sdk/shopify-oxygen",
+    "packages/sdk/shopify-oxygen/contract-tests"
   ],
   "private": true,
   "scripts": {

packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts

@@ -70,7 +70,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that is i
     await expect(client.waitForInitialization()).resolves.toBeUndefined();
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 });
@@ -83,7 +83,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init
     mockBrowserClient.identify.mockImplementation(
       () =>
         new Promise((r) => {
-          setTimeout(r, 100);
+          setTimeout(() => r({ status: 'completed' }), 100);
         }),
     );
     client = new LDClientCompatImpl('env-key', { kind: 'user', key: 'user-key' });
@@ -99,7 +99,10 @@ describe('given a LDClientCompatImpl client with mocked browser client that init
 
     const result = await client.identify(context);
 
-    expect(mockBrowserClient.identify).toHaveBeenCalledWith(context, { hash: undefined });
+    expect(mockBrowserClient.identify).toHaveBeenCalledWith(context, {
+      hash: undefined,
+      sheddable: false,
+    });
     expect(result).toEqual(mockFlags);
   });
 
@@ -168,7 +171,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init
     await expect(client.waitForInitialization()).resolves.toBeUndefined();
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -180,7 +183,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init
     await expect(client.waitForInitialization()).resolves.toBeUndefined();
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -191,7 +194,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init
     await expect(client.waitUntilReady()).resolves.toBeUndefined();
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -429,6 +432,7 @@ it('forwards bootstrap and hash to BrowserClient identify call', async () => {
     bootstrap: bootstrapData,
     hash: 'someHash',
     noTimeout: true,
+    sheddable: false,
   });
 });
 
@@ -453,7 +457,7 @@ describe('given a LDClientCompatImpl client with mocked browser client which fai
 
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -466,7 +470,7 @@ describe('given a LDClientCompatImpl client with mocked browser client which fai
 
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -477,7 +481,7 @@ describe('given a LDClientCompatImpl client with mocked browser client which fai
 
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 
@@ -489,7 +493,7 @@ describe('given a LDClientCompatImpl client with mocked browser client which fai
 
     expect(mockBrowserClient.identify).toHaveBeenCalledWith(
       { kind: 'user', key: 'user-key' },
-      { bootstrap: undefined, hash: undefined, noTimeout: true },
+      { bootstrap: undefined, hash: undefined, noTimeout: true, sheddable: false },
     );
   });
 

packages/sdk/browser/src/compat/LDClientCompatImpl.ts

@@ -54,7 +54,20 @@ export default class LDClientCompatImpl implements LDClient {
     hash?: string,
   ): Promise<void> {
     try {
-      await this._client.identify(context, { noTimeout: true, bootstrap, hash });
+      const result = await this._client.identify(context, {
+        noTimeout: true,
+        bootstrap,
+        hash,
+        sheddable: false,
+      });
+
+      if (result.status === 'error') {
+        throw result.error;
+      } else if (result.status === 'timeout') {
+        throw new LDTimeoutError('Identify timed out');
+      }
+      // status === 'completed' ('shed' cannot happen with sheddable: false)
+
       this._initState = 'success';
       this._initResolve?.();
       this._emitter.emit('initialized');
@@ -138,7 +151,16 @@ export default class LDClientCompatImpl implements LDClient {
     onDone?: (err: Error | null, flags: LDFlagSet | null) => void,
   ): Promise<LDFlagSet> | undefined {
     return wrapPromiseCallback(
-      this._client.identify(context, { hash }).then(() => this.allFlags()),
+      this._client.identify(context, { hash, sheddable: false }).then((result) => {
+        // Check if identification was successful
+        if (result.status === 'error') {
+          throw result.error;
+        } else if (result.status === 'timeout') {
+          throw new LDTimeoutError('Identify timed out');
+        }
+        // status === 'completed' ('shed' cannot happen with sheddable: false)
+        return this.allFlags();
+      }),
       onDone,
     ) as Promise<LDFlagSet> | undefined;
     // The typing here is a little funny. The wrapPromiseCallback can technically return

packages/sdk/shopify-oxygen/contract-tests/.gitignore

@@ -0,0 +1 @@
+*.log

packages/sdk/shopify-oxygen/contract-tests/package.json

@@ -0,0 +1,24 @@
+{
+  "name": "@launchdarkly/shopify-oxygen-contract-tests",
+  "version": "0.0.0",
+  "main": "dist/index.js",
+  "scripts": {
+    "start": "node --inspect dist/index.js",
+    "build": "tsup",
+    "dev": "tsc --watch"
+  },
+  "type": "module",
+  "license": "Apache-2.0",
+  "private": true,
+  "dependencies": {
+    "@launchdarkly/js-server-sdk-common": "workspace:^",
+    "@launchdarkly/shopify-oxygen-sdk": "workspace:^",
+    "express": "^5.0.1"
+  },
+  "devDependencies": {
+    "@types/express": "^5.0.1",
+    "@types/node": "^18.11.9",
+    "tsup": "^8.5.0",
+    "typescript": "^4.9.0"
+  }
+}

packages/sdk/shopify-oxygen/contract-tests/run-test-harness.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# If sdk-test-harness is not in the path, then you will need to set
+# the SDK_TEST_HARNESS environment variable to the path to the sdk-test-harness binary.
+
+# Default to the local sdk-test-harness binary if not provided
+if [ -z "${SDK_TEST_HARNESS}" ]; then
+  SDK_TEST_HARNESS="sdk-test-harness"
+fi
+
+# Uncomment this to start the test service in the background
+# yarn start &> test-service.log &
+
+# skipping all tests that require streaming connections.
+${SDK_TEST_HARNESS} --url http://localhost:8000 \
+  --skip "streaming.*" \
+  --skip "evaluation.*" \
+  --skip "event.*" \
+  --skip "service.*" \
+  --stop-service-at-end

packages/sdk/shopify-oxygen/contract-tests/src/index.ts

@@ -0,0 +1,68 @@
+import express, { Request, Response } from 'express';
+import { Server } from 'http';
+
+import { ClientPool } from './utils';
+
+/* eslint-disable no-console */
+
+// export DEBUG=true to enable debugging
+// unset DEBUG to disable debugging
+const debugging = process.env.DEBUG === 'true';
+
+const app = express();
+let server: Server | null = null;
+
+app.use(express.json());
+
+const port = 8000;
+
+const clientPool = new ClientPool();
+
+if (debugging) {
+  app.use((req: Request, res: Response, next: Function) => {
+    console.debug('request', req.method, req.url);
+    if (req.body) {
+      console.debug('request', JSON.stringify(req.body, null, 2));
+    }
+    next();
+  });
+} else {
+  // NOOP global console.debug
+  console.debug = () => {};
+}
+
+app.get('/', (req: Request, res: Response) => {
+  res.header('Content-Type', 'application/json');
+  res.json({
+    capabilities: ['server-side-polling', 'server-side'],
+  });
+});
+
+app.delete('/', (req: Request, res: Response) => {
+  console.log('Test service has told us to exit');
+  res.status(204);
+  res.send();
+
+  if (server) {
+    server.close(() => process.exit());
+  }
+});
+
+app.post('/', async (req: Request, res: Response) => {
+  await clientPool.createClient(req.body, res);
+});
+
+app.post('/clients/:id', async (req: Request, res: Response) => {
+  await clientPool.runCommand(req.params.id, req.body, res);
+});
+
+app.delete('/clients/:id', async (req: Request, res: Response) => {
+  console.debug('DELETE request received /clients/:id');
+  console.debug(req.params.id);
+  await clientPool.deleteClient(req.params.id, res);
+});
+
+server = app.listen(port, () => {
+  // eslint-disable-next-line no-console
+  console.log('Listening on port %d', port);
+});