Merge pull request #545 from ForgeRock/fix-jc-return-types

fix(journey-client): remove undefined from JourneyResult return type

Author: ryanbas21

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

@@ -99,13 +99,10 @@ describe('journey-client', () => {
   });
 
   test('journey_WellknownConfig_ReturnsClientWithAllMethods', async () => {
-    // Arrange
     setupMockFetch();
 
-    // Act
     const client = await journey({ config: mockConfig });
 
-    // Assert
     expect(client.start).toBeInstanceOf(Function);
     expect(client.next).toBeInstanceOf(Function);
     expect(client.redirect).toBeInstanceOf(Function);
@@ -114,39 +111,32 @@ describe('journey-client', () => {
   });
 
   test('journey_InvalidWellknownUrl_ThrowsError', async () => {
-    // Arrange
     const invalidConfig: JourneyClientConfig = {
       serverConfig: {
         wellknown: 'not-a-valid-url',
       },
     };
 
-    // Act & Assert
     await expect(journey({ config: invalidConfig })).rejects.toThrow('Invalid wellknown URL');
   });
 
   test('journey_MissingWellknownPath_ThrowsError', async () => {
-    // Arrange — valid HTTPS URL but missing /.well-known/openid-configuration
     const badPathConfig: JourneyClientConfig = {
       serverConfig: {
         wellknown: 'https://am.example.com/am/oauth2/alpha',
       },
     };
 
-    // Act & Assert
     await expect(journey({ config: badPathConfig })).rejects.toThrow('Invalid wellknown URL');
   });
 
   test('start_WellknownConfig_FetchesFirstStep', async () => {
-    // Arrange
     const mockStepResponse: Step = { authId: 'test-auth-id', callbacks: [] };
     setupMockFetch(mockStepResponse);
 
-    // Act
     const client = await journey({ config: mockConfig });
     const step = await client.start();
 
-    // Assert
     expect(step).toBeDefined();
     expect(isGenericError(step)).toBe(false);
 
@@ -163,7 +153,6 @@ describe('journey-client', () => {
   });
 
   test('next_WellknownConfig_SendsStepAndReturnsNext', async () => {
-    // Arrange
     const initialStep = createJourneyStep({
       authId: 'test-auth-id',
       callbacks: [
@@ -186,11 +175,9 @@ describe('journey-client', () => {
     };
     setupMockFetch(nextStepPayload);
 
-    // Act
     const client = await journey({ config: mockConfig });
     const nextStep = await client.next(initialStep, {});
 
-    // Assert
     expect(nextStep).toBeDefined();
     expect(isGenericError(nextStep)).toBe(false);
 
@@ -206,7 +193,6 @@ describe('journey-client', () => {
   });
 
   test('redirect_WellknownConfig_StoresStepAndCallsLocationAssign', async () => {
-    // Arrange
     const mockStepPayload: Step = {
       callbacks: [
         {
@@ -224,11 +210,9 @@ describe('journey-client', () => {
     });
     setupMockFetch();
 
-    // Act
     const client = await journey({ config: mockConfig });
     await client.redirect(step);
 
-    // Assert
     expect(mockStorageInstance.set).toHaveBeenCalledWith({ step: step.payload });
     expect(assignMock).toHaveBeenCalledWith('https://sso.com/redirect');
 
@@ -237,20 +221,17 @@ describe('journey-client', () => {
 
   describe('resume()', () => {
     test('resume_WithPreviousStepInStorage_CallsNextWithUrlParams', async () => {
-      // Arrange
       const previousStepPayload: Step = {
         callbacks: [{ type: callbackType.RedirectCallback, input: [], output: [] }],
       };
       mockStorageInstance.get.mockResolvedValue({ step: previousStepPayload });
       const nextStepPayload: Step = { authId: 'test-auth-id', callbacks: [] };
       setupMockFetch(nextStepPayload);
 
-      // Act
       const client = await journey({ config: mockConfig });
       const resumeUrl = 'https://app.com/callback?code=123&state=abc';
       const step = await client.resume(resumeUrl, {});
 
-      // Assert
       expect(step).toBeDefined();
       expect(mockStorageInstance.get).toHaveBeenCalledTimes(1);
       expect(mockStorageInstance.remove).toHaveBeenCalledTimes(1);
@@ -269,7 +250,6 @@ describe('journey-client', () => {
     });
 
     test('resume_WithPlainStepObjectInStorage_CorrectlyResumes', async () => {
-      // Arrange
       const plainStepPayload: Step = {
         callbacks: [
           { type: callbackType.TextOutputCallback, output: [{ name: 'message', value: 'Hello' }] },
@@ -280,12 +260,10 @@ describe('journey-client', () => {
       const nextStepPayload: Step = { authId: 'test-auth-id', callbacks: [] };
       setupMockFetch(nextStepPayload);
 
-      // Act
       const client = await journey({ config: mockConfig });
       const resumeUrl = 'https://app.com/callback?code=123&state=abc';
       const step = await client.resume(resumeUrl, {});
 
-      // Assert
       expect(step).toBeDefined();
       expect(mockStorageInstance.get).toHaveBeenCalledTimes(1);
       expect(mockStorageInstance.remove).toHaveBeenCalledTimes(1);
@@ -300,15 +278,12 @@ describe('journey-client', () => {
     });
 
     test('resume_PreviousStepRequiredButNotFound_ThrowsError', async () => {
-      // Arrange
       mockStorageInstance.get.mockResolvedValue(undefined);
       setupMockFetch();
 
-      // Act
       const client = await journey({ config: mockConfig });
       const resumeUrl = 'https://app.com/callback?code=123&state=abc';
 
-      // Assert
       await expect(client.resume(resumeUrl)).rejects.toThrow(
         'Error: previous step information not found in storage for resume operation.',
       );
@@ -317,17 +292,14 @@ describe('journey-client', () => {
     });
 
     test('resume_NoPreviousStepRequired_CallsStartWithUrlParams', async () => {
-      // Arrange
       mockStorageInstance.get.mockResolvedValue(undefined);
       const mockStepResponse: Step = { authId: 'test-auth-id', callbacks: [] };
       setupMockFetch(mockStepResponse);
 
-      // Act
       const client = await journey({ config: mockConfig });
       const resumeUrl = 'https://app.com/callback?foo=bar';
       const step = await client.resume(resumeUrl, {});
 
-      // Assert
       expect(step).toBeDefined();
       expect(mockStorageInstance.get).not.toHaveBeenCalled();
       expect(mockFetch).toHaveBeenCalledTimes(2); // wellknown + start
@@ -341,9 +313,21 @@ describe('journey-client', () => {
     });
   });
 
+  test('start_NoDataFromServer_ReturnsGenericError', async () => {
+    setupMockFetch(null);
+
+    const client = await journey({ config: mockConfig });
+    const result = await client.start();
+
+    expect(isGenericError(result)).toBe(true);
+    if (isGenericError(result)) {
+      expect(result.error).toBe('no_response_data');
+      expect(result.type).toBe('unknown_error');
+    }
+  });
+
   describe('baseUrl from convertWellknown', () => {
     test('journey_LocalhostWellknown_ConstructsCorrectUrls', async () => {
-      // Arrange
       const localhostConfig: JourneyClientConfig = {
         serverConfig: {
           wellknown: 'http://localhost:9443/am/oauth2/realms/root/.well-known/openid-configuration',
@@ -366,11 +350,9 @@ describe('journey-client', () => {
         return Promise.resolve(new Response(JSON.stringify(mockStepResponse)));
       });
 
-      // Act
       const client = await journey({ config: localhostConfig });
       await client.start();
 
-      // Assert
       expect(mockFetch).toHaveBeenCalledTimes(2);
       const request = mockFetch.mock.calls[1][0] as Request;
       expect(request.url).toBe('http://localhost:9443/am/json/realms/root/authenticate');
@@ -379,7 +361,6 @@ describe('journey-client', () => {
 
   describe('subrealm inference', () => {
     test('journey_WellknownWithSubrealm_DerivesCorrectPaths', async () => {
-      // Arrange
       const alphaConfig: JourneyClientConfig = {
         serverConfig: {
           wellknown:
@@ -404,11 +385,9 @@ describe('journey-client', () => {
         return Promise.resolve(new Response(JSON.stringify(mockStepResponse)));
       });
 
-      // Act
       const client = await journey({ config: alphaConfig });
       await client.start();
 
-      // Assert
       const request = mockFetch.mock.calls[1][0] as Request;
       expect(request.url).toBe('https://test.com/am/json/realms/root/realms/alpha/authenticate');
     });

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

@@ -32,12 +32,7 @@ import type { JourneyLoginFailure } from './login-failure.utils.js';
 import type { JourneyLoginSuccess } from './login-success.utils.js';
 
 /** Result type for journey client methods. */
-type JourneyResult =
-  | JourneyStep
-  | JourneyLoginSuccess
-  | JourneyLoginFailure
-  | GenericError
-  | undefined;
+type JourneyResult = JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericError;
 
 /** The journey client instance returned by the `journey()` function. */
 export interface JourneyClient {

packages/journey-client/src/lib/client.types.test-d.ts

@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
+ *
+ * This software may be modified and distributed under the terms
+ * of the MIT license. See the LICENSE file for details.
+ */
+/* eslint-disable @typescript-eslint/no-unused-vars */
+import { describe, it } from 'vitest';
+
+import type { GenericError } from '@forgerock/sdk-types';
+
+import type { JourneyClient } from './client.types.js';
+import type { JourneyStep } from './step.utils.js';
+import type { JourneyLoginSuccess } from './login-success.utils.js';
+import type { JourneyLoginFailure } from './login-failure.utils.js';
+
+/**
+ * Resolves to `true` if `U` is a member of union `T`, `false` otherwise.
+ * Uses the distributive-conditional-type trick: when `U` is a union member
+ * of `T`, `U extends T` distributes and resolves to `true`.
+ */
+type HasMember<T, U> = U extends T ? true : false;
+
+/** Compile-time assertion: `T` must be exactly `true`. */
+type AssertTrue<T extends true> = T;
+
+/** Unwrap Promise<T> → T. */
+type Awaited<T> = T extends Promise<infer U> ? U : T;
+
+type StartResult = Awaited<ReturnType<JourneyClient['start']>>;
+type NextResult = Awaited<ReturnType<JourneyClient['next']>>;
+type ResumeResult = Awaited<ReturnType<JourneyClient['resume']>>;
+type TerminateResult = Awaited<ReturnType<JourneyClient['terminate']>>;
+
+describe('JourneyClient return types', () => {
+  it('start includes all expected members and excludes undefined', () => {
+    type _hasStep = AssertTrue<HasMember<StartResult, JourneyStep>>;
+    type _hasSuccess = AssertTrue<HasMember<StartResult, JourneyLoginSuccess>>;
+    type _hasFailure = AssertTrue<HasMember<StartResult, JourneyLoginFailure>>;
+    type _hasError = AssertTrue<HasMember<StartResult, GenericError>>;
+    type _noUndefined = AssertTrue<HasMember<StartResult, undefined> extends false ? true : false>;
+  });
+
+  it('next includes all expected members and excludes undefined', () => {
+    type _hasStep = AssertTrue<HasMember<NextResult, JourneyStep>>;
+    type _hasSuccess = AssertTrue<HasMember<NextResult, JourneyLoginSuccess>>;
+    type _hasFailure = AssertTrue<HasMember<NextResult, JourneyLoginFailure>>;
+    type _hasError = AssertTrue<HasMember<NextResult, GenericError>>;
+    type _noUndefined = AssertTrue<HasMember<NextResult, undefined> extends false ? true : false>;
+  });
+
+  it('resume includes all expected members and excludes undefined', () => {
+    type _hasStep = AssertTrue<HasMember<ResumeResult, JourneyStep>>;
+    type _hasSuccess = AssertTrue<HasMember<ResumeResult, JourneyLoginSuccess>>;
+    type _hasFailure = AssertTrue<HasMember<ResumeResult, JourneyLoginFailure>>;
+    type _hasError = AssertTrue<HasMember<ResumeResult, GenericError>>;
+    type _noUndefined = AssertTrue<HasMember<ResumeResult, undefined> extends false ? true : false>;
+  });
+
+  it('terminate returns void | GenericError', () => {
+    type _hasVoid = AssertTrue<HasMember<TerminateResult, void>>;
+    type _hasError = AssertTrue<HasMember<TerminateResult, GenericError>>;
+  });
+});

packages/journey-client/src/lib/config.slice.test.ts

@@ -27,15 +27,12 @@ function createMockWellknown(overrides: Partial<WellknownResponse> = {}): Wellkn
 describe('journey-client config.slice', () => {
   describe('configSlice_ValidAmWellknown_SetsResolvedServerConfig', () => {
     it('should derive baseUrl and paths from a standard AM well-known response', () => {
-      // Arrange
       const payload: ResolvedConfig = {
         wellknownResponse: createMockWellknown(),
       };
 
-      // Act
       const state = configSlice.reducer(undefined, configSlice.actions.set(payload));
 
-      // Assert
       expect(state.serverConfig).toEqual({
         baseUrl: 'https://am.example.com',
         paths: {
@@ -49,18 +46,15 @@ describe('journey-client config.slice', () => {
 
   describe('configSlice_NonAmIssuer_SetsError', () => {
     it('should set a GenericError when the issuer is not a ForgeRock AM issuer', () => {
-      // Arrange
       const payload: ResolvedConfig = {
         wellknownResponse: createMockWellknown({
           issuer: 'https://auth.pingone.com/env-id/as',
           authorization_endpoint: 'https://auth.pingone.com/env-id/as/authorize',
         }),
       };
 
-      // Act
       const state = configSlice.reducer(undefined, configSlice.actions.set(payload));
 
-      // Assert
       expect(state.error).toBeDefined();
       expect(state.error?.type).toBe('wellknown_error');
       expect(state.error?.message).toContain('ForgeRock AM issuer');
@@ -69,17 +63,14 @@ describe('journey-client config.slice', () => {
 
   describe('configSlice_MissingAuthEndpoint_SetsError', () => {
     it('should set a GenericError when authorization_endpoint is empty', () => {
-      // Arrange
       const payload: ResolvedConfig = {
         wellknownResponse: createMockWellknown({
           authorization_endpoint: '',
         }),
       };
 
-      // Act
       const state = configSlice.reducer(undefined, configSlice.actions.set(payload));
 
-      // Assert
       expect(state.error).toBeDefined();
       expect(state.error?.type).toBe('wellknown_error');
       expect(state.error?.message).toContain('authorization_endpoint');