fix!: update-error-handling-across-packages

.changeset/chilly-sheep-follow.md

@@ -0,0 +1,40 @@
+---
+'@forgerock/storage': major
+'@forgerock/sdk-oidc': major
+'@forgerock/journey-client': major
+'@forgerock/oidc-client': major
+---
+
+**BREAKING CHANGE**: Journey client methods now return `GenericError` instead of `undefined` for error cases.
+
+## What Changed
+
+The `start`, `next`, `resume`, and `terminate` methods now return a `GenericError` object instead of `undefined` when encountering unknown step types or error conditions. This aligns the journey client with the DaVinci client's error handling patterns.
+
+### Return Type Changes
+
+| Method      | Before                                                                   | After                                                                       |
+| ----------- | ------------------------------------------------------------------------ | --------------------------------------------------------------------------- |
+| `start`     | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| undefined` | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| GenericError` |
+| `next`      | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| undefined` | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| GenericError` |
+| `resume`    | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| undefined` | `JourneyStep \| JourneyLoginSuccess \| JourneyLoginFailure \| GenericError` |
+| `terminate` | `void`                                                                   | `void \| GenericError`                                                      |
+
+## Migration Guide
+
+Before:
+
+```ts
+const step = await client.start();
+if (step) {
+  // Use step
+}
+
+After: const result = await client.start();
+if ('error' in result) {
+  // Handle error
+  console.error(result.message);
+} else {
+  // Use step
+}
+```

packages/journey-client/src/lib/client.store.test.ts

@@ -8,12 +8,19 @@
 import { callbackType } from '@forgerock/sdk-types';
 import { afterEach, describe, expect, test, vi } from 'vitest';
 
-import type { Step } from '@forgerock/sdk-types';
+import type { GenericError, Step } from '@forgerock/sdk-types';
 
 import { journey } from './client.store.js';
 import { createJourneyStep } from './step.utils.js';
 import { JourneyClientConfig } from './config.types.js';
 
+/**
+ * Type guard to check if a result is a GenericError
+ */
+function isGenericError(result: unknown): result is GenericError {
+  return typeof result === 'object' && result !== null && 'error' in result && 'type' in result;
+}
+
 // Create a singleton mock instance for storage
 const mockStorageInstance = {
   get: vi.fn(),
@@ -65,13 +72,16 @@ describe('journey-client', () => {
     const client = await journey({ config: mockConfig });
     const step = await client.start();
     expect(step).toBeDefined();
+    expect(isGenericError(step)).toBe(false);
 
     expect(mockFetch).toHaveBeenCalledTimes(1);
     const request = mockFetch.mock.calls[0][0] as Request;
     // TODO: This should be /journeys?_action=start, but the current implementation calls /authenticate
     expect(request.url).toBe('https://test.com/json/realms/root/authenticate');
     expect(step).toHaveProperty('type', 'Step');
-    expect(step && step.payload).toEqual(mockStepResponse);
+    if (!isGenericError(step)) {
+      expect(step.payload).toEqual(mockStepResponse);
+    }
   });
 
   test('next() should send the current step and return the next step', async () => {
@@ -101,6 +111,7 @@ describe('journey-client', () => {
     const client = await journey({ config: mockConfig });
     const nextStep = await client.next(initialStep, {});
     expect(nextStep).toBeDefined();
+    expect(isGenericError(nextStep)).toBe(false);
 
     expect(mockFetch).toHaveBeenCalledTimes(1);
     const request = mockFetch.mock.calls[0][0] as Request;
@@ -109,7 +120,9 @@ describe('journey-client', () => {
     expect(request.method).toBe('POST');
     expect(await request.json()).toEqual(initialStep.payload);
     expect(nextStep).toHaveProperty('type', 'Step');
-    expect(nextStep && nextStep.payload).toEqual(nextStepPayload);
+    if (!isGenericError(nextStep)) {
+      expect(nextStep.payload).toEqual(nextStepPayload);
+    }
   });
 
   test('redirect() should store the step and call location.assign', async () => {
@@ -169,7 +182,9 @@ describe('journey-client', () => {
       expect(request.method).toBe('POST');
       expect(await request.json()).toEqual(previousStepPayload);
       expect(step).toHaveProperty('type', 'Step');
-      expect(step && step.payload).toEqual(nextStepPayload);
+      if (!isGenericError(step)) {
+        expect(step.payload).toEqual(nextStepPayload);
+      }
     });
 
     test('should correctly resume with a plain Step object from storage', async () => {
@@ -198,7 +213,9 @@ describe('journey-client', () => {
       expect(request.method).toBe('POST');
       expect(await request.json()).toEqual(plainStepPayload); // Expect the plain payload to be sent
       expect(step).toHaveProperty('type', 'Step'); // The returned step should still be an JourneyStep instance
-      expect(step && step.payload).toEqual(nextStepPayload);
+      if (!isGenericError(step)) {
+        expect(step.payload).toEqual(nextStepPayload);
+      }
     });
 
     test('should throw an error if a previous step is required but not found', async () => {
@@ -234,7 +251,9 @@ describe('journey-client', () => {
       const url = new URL(request.url);
       expect(url.origin + url.pathname).toBe('https://test.com/json/realms/root/authenticate');
       expect(step).toHaveProperty('type', 'Step');
-      expect(step && step.payload).toEqual(mockStepResponse);
+      if (!isGenericError(step)) {
+        expect(step.payload).toEqual(mockStepResponse);
+      }
     });
   });
 

packages/journey-client/src/lib/client.store.ts

@@ -70,19 +70,36 @@ export async function journey({
   const self = {
     start: async (options?: StartParam) => {
       const { data } = await store.dispatch(journeyApi.endpoints.start.initiate(options));
-      return data ? createJourneyObject(data) : undefined;
+      if (!data) {
+        const error: GenericError = {
+          error: 'no_response_data',
+          message: 'No data received from server when starting journey',
+          type: 'unknown_error',
+        };
+        return error;
+      }
+      return createJourneyObject(data);
     },
 
     /**
      * Submits the current Step payload to the authentication API and retrieves the next JourneyStep in the journey.
      * The `step` to be submitted is provided within the `options` object.
      *
-     * @param options An object containing the current Step payload and optional JourneyClientConfig.
-     * @returns A Promise that resolves to the next JourneyStep in the journey, or undefined if the journey ends.
+     * @param step The current JourneyStep containing user input to submit.
+     * @param options Optional configuration for the request.
+     * @returns A Promise that resolves to a JourneyStep, JourneyLoginSuccess, JourneyLoginFailure, or GenericError.
      */
     next: async (step: JourneyStep, options?: NextOptions) => {
       const { data } = await store.dispatch(journeyApi.endpoints.next.initiate({ step, options }));
-      return data ? createJourneyObject(data) : undefined;
+      if (!data) {
+        const error: GenericError = {
+          error: 'no_response_data',
+          message: 'No data received from server when submitting step',
+          type: 'unknown_error',
+        };
+        return error;
+      }
+      return createJourneyObject(data);
     },
 
     // TODO: Remove the actual redirect from this method and just return the URL to the caller
@@ -108,7 +125,7 @@ export async function journey({
     resume: async (
       url: string,
       options?: ResumeOptions,
-    ): Promise<ReturnType<typeof createJourneyObject> | undefined> => {
+    ): Promise<ReturnType<typeof createJourneyObject>> => {
       const parsedUrl = new URL(url);
       const code = parsedUrl.searchParams.get('code');
       const state = parsedUrl.searchParams.get('state');
@@ -183,11 +200,22 @@ export async function journey({
      * This will invalidate the current session and clean up any server-side state.
      *
      * @param options Optional StepOptions containing query parameters.
-     * @returns A Promise that resolves when the session is successfully ended.
+     * @returns A Promise that resolves to void on success, or GenericError on failure.
      */
-    terminate: async (options?: { query?: Record<string, string> }) => {
-      const { data } = await store.dispatch(journeyApi.endpoints.terminate.initiate(options));
-      return data;
+    terminate: async (options?: {
+      query?: Record<string, string>;
+    }): Promise<void | GenericError> => {
+      const { error } = await store.dispatch(journeyApi.endpoints.terminate.initiate(options));
+      if (error) {
+        return {
+          error: 'terminate_failed',
+          message:
+            'status' in error
+              ? `Failed to terminate session: ${error.status}`
+              : 'Failed to terminate session',
+          type: 'unknown_error',
+        };
+      }
     },
   };
   return self;

packages/journey-client/src/lib/device/device-profile.test.ts

@@ -7,7 +7,7 @@
  * This software may be modified and distributed under the terms
  * of the MIT license. See the LICENSE file for details.
  */
-import { vi, expect, describe, it, afterEach, beforeEach, SpyInstance } from 'vitest';
+import { vi, expect, describe, it, afterEach, beforeEach, MockInstance } from 'vitest';
 
 import { Device } from './device-profile.js';
 
@@ -86,7 +86,7 @@ describe('Test DeviceProfile', () => {
   });
 
   describe('logLevel tests', () => {
-    let warnSpy: SpyInstance;
+    let warnSpy: MockInstance;
     const originalNavigator = global.navigator;
 
     beforeEach(() => {

packages/journey-client/src/lib/journey.utils.ts

@@ -7,7 +7,7 @@
 
 import { StepType } from '@forgerock/sdk-types';
 
-import type { Step } from '@forgerock/sdk-types';
+import type { GenericError, Step } from '@forgerock/sdk-types';
 
 import { createJourneyLoginSuccess } from './login-success.utils.js';
 import { createJourneyLoginFailure } from './login-failure.utils.js';
@@ -17,9 +17,17 @@ import type { JourneyStep } from './step.utils.js';
 import type { JourneyLoginFailure } from './login-failure.utils.js';
 import type { JourneyLoginSuccess } from './login-success.utils.js';
 
+/**
+ * Creates a journey object from a raw Step response.
+ * Determines the step type based on the presence of authId or successUrl properties
+ * and returns the appropriate journey object type.
+ *
+ * @param step - The raw Step response from the authentication API
+ * @returns A JourneyStep, JourneyLoginSuccess, JourneyLoginFailure, or GenericError if the step type cannot be determined
+ */
 function createJourneyObject(
   step: Step,
-): JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | undefined {
+): JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericError {
   let type;
   if (step.authId) {
     type = StepType.Step;
@@ -37,7 +45,11 @@ function createJourneyObject(
     case StepType.Step:
       return createJourneyStep(step);
     default:
-      return undefined;
+      return {
+        error: 'unknown_step_type',
+        message: 'Unable to determine step type from server response',
+        type: 'unknown_error',
+      };
   }
 }