Skip to content

fix(journey-client): create JourneyLoginFailure step, handle Login Failure case #574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
vatsalparikh wants to merge 5 commits into
base: main
Choose a base branch
from
+236 −45
Draft

fix(journey-client): create JourneyLoginFailure step, handle Login Failure case #574

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f66b1fd
fix(journey-client): create JourneyLoginFailure step and handle Login…
vatsalparikh Apr 20, 2026
13326e0
fix(journey-client): create JourneyLoginFailure step and handle Login…
vatsalparikh Apr 20, 2026
6708610
fix(journey-client): handle errors in journey utils with RTK narrowing
vatsalparikh Apr 22, 2026
dcf367f
fix(journey-client): treat object error.data as LoginFailure and remo…
vatsalparikh May 7, 2026
4b030d5
fix(journey-client): error handling in journey.api file to keep infra…
vatsalparikh May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/whole-mangos-find.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@forgerock/journey-client': patch
---

Return `JourneyLoginFailure` by hitting the previously-unreached `LoginFailure` branch when `start()`/`next()` receives a failure payload with a login failure `code`
1 change: 0 additions & 1 deletion e2e/journey-app/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ if (searchParams.get('middleware') === 'true') {
renderComplete();
} else if (step?.type === 'LoginFailure') {
console.error('Journey failed');
renderForm();
renderError();
} else {
console.error('Unknown node status', step);
Expand Down
14 changes: 7 additions & 7 deletions packages/davinci-client/api-report/davinci-client.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
resume: (input: {
continueToken: string;
}) => Promise<InternalErrorResponse | NodeStates>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
poll: (collector: PollingCollector) => Poller;
pollStatus: (collector: PollingCollector) => Poller;
getClient: () => {
status: "start";
} | {
action: string;
collectors: Collectors[];
description?: string;
Expand All @@ -287,6 +285,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
status: "error";
} | {
status: "failure";
} | {
status: "start";
} | {
authorization?: {
code?: string;
Expand All @@ -297,7 +297,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
getCollectors: () => Collectors[];
getError: () => DaVinciError | null;
getErrorCollectors: () => CollectorErrors[];
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
getServer: () => {
_links?: Links;
id?: string;
Expand All @@ -306,8 +306,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
href?: string;
eventName?: string;
status: "continue";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand All @@ -323,6 +321,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
interactionId?: string;
interactionToken?: string;
status: "failure";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand Down
14 changes: 7 additions & 7 deletions packages/davinci-client/api-report/davinci-client.types.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
resume: (input: {
continueToken: string;
}) => Promise<InternalErrorResponse | NodeStates>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
poll: (collector: PollingCollector) => Poller;
pollStatus: (collector: PollingCollector) => Poller;
getClient: () => {
status: "start";
} | {
action: string;
collectors: Collectors[];
description?: string;
Expand All @@ -287,6 +285,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
status: "error";
} | {
status: "failure";
} | {
status: "start";
} | {
authorization?: {
code?: string;
Expand All @@ -297,7 +297,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
getCollectors: () => Collectors[];
getError: () => DaVinciError | null;
getErrorCollectors: () => CollectorErrors[];
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
getServer: () => {
_links?: Links;
id?: string;
Expand All @@ -306,8 +306,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
href?: string;
eventName?: string;
status: "continue";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand All @@ -323,6 +321,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
interactionId?: string;
interactionToken?: string;
status: "failure";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand Down
3 changes: 3 additions & 0 deletions packages/journey-client/api-report/journey-client.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { RequestMiddleware } from '@forgerock/sdk-request-middleware';
import { Step } from '@forgerock/sdk-types';
import { StepDetail } from '@forgerock/sdk-types';
import { StepType } from '@forgerock/sdk-types';
import { WellknownResponse } from '@forgerock/sdk-types';

export { ActionTypes }

Expand Down Expand Up @@ -494,6 +495,8 @@ export class ValidatedCreateUsernameCallback extends BaseCallback {
setValidateOnly(value: boolean): void;
}

export { WellknownResponse }

// (No @packageDocumentation comment for this package)

```
3 changes: 3 additions & 0 deletions packages/journey-client/api-report/journey-client.types.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { RequestMiddleware } from '@forgerock/sdk-request-middleware';
import { Step } from '@forgerock/sdk-types';
import { StepDetail } from '@forgerock/sdk-types';
import { StepType } from '@forgerock/sdk-types';
import { WellknownResponse } from '@forgerock/sdk-types';

export { ActionTypes }

Expand Down Expand Up @@ -481,6 +482,8 @@ export class ValidatedCreateUsernameCallback extends BaseCallback {
setValidateOnly(value: boolean): void;
}

export { WellknownResponse }

// (No @packageDocumentation comment for this package)

```
83 changes: 75 additions & 8 deletions packages/journey-client/src/lib/client.store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
// @vitest-environment node
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
* Copyright (c) 2025-2026 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.
*/

import { callbackType } from '@forgerock/sdk-types';
import { afterEach, describe, expect, test, vi } from 'vitest';

import type { GenericError, Step, WellknownResponse } from '@forgerock/sdk-types';

import { journey } from './client.store.js';
import { createJourneyStep } from './step.utils.js';

import { callbackType, type GenericError, type Step, type WellknownResponse } from '../index.js';

import { JourneyClientConfig } from './config.types.js';

/**
Expand Down Expand Up @@ -75,7 +76,7 @@ function getUrlFromInput(input: RequestInfo | URL): string {
/**
* Helper to setup mock fetch for wellknown + journey responses
*/
function setupMockFetch(journeyResponse: Step | null = null) {
function setupMockFetch(journeyResponse: Step | null = null, authenticateStatus = 200) {
mockFetch.mockImplementation((input: RequestInfo | URL) => {
const url = getUrlFromInput(input);

Expand All @@ -85,8 +86,13 @@ function setupMockFetch(journeyResponse: Step | null = null) {
}

// Journey authenticate endpoint
if (journeyResponse && url.includes('/authenticate')) {
return Promise.resolve(new Response(JSON.stringify(journeyResponse)));
if (url.includes('/authenticate')) {
if (journeyResponse === null) {
return Promise.reject(new Error(`Unexpected fetch: ${url}`));
}
return Promise.resolve(
new Response(JSON.stringify(journeyResponse), { status: authenticateStatus }),
);
}

return Promise.reject(new Error(`Unexpected fetch: ${url}`));
Expand Down Expand Up @@ -152,6 +158,30 @@ describe('journey-client', () => {
}
});

test('start_401WithStepPayload_ReturnsLoginFailure', async () => {
const failurePayload: Step = {
code: 401,
message: 'Access Denied',
reason: 'Unauthorized',
detail: { failureUrl: 'https://example.com/failure' },
};
setupMockFetch(failurePayload, 401);

const client = await journey({ config: mockConfig });
const result = await client.start();

expect(result).toBeDefined();
expect(isGenericError(result)).toBe(false);
expect(result).toHaveProperty('type', 'LoginFailure');

if (!isGenericError(result) && result.type === 'LoginFailure') {
expect(result.payload).toEqual(failurePayload);
expect(result.getCode()).toBe(401);
expect(result.getMessage()).toBe('Access Denied');
expect(result.getReason()).toBe('Unauthorized');
}
});

test('next_WellknownConfig_SendsStepAndReturnsNext', async () => {
const initialStep = createJourneyStep({
authId: 'test-auth-id',
Expand Down Expand Up @@ -192,6 +222,34 @@ describe('journey-client', () => {
}
});

test('next_401WithStepPayload_ReturnsLoginFailure', async () => {
const initialStep = createJourneyStep({
authId: 'test-auth-id',
callbacks: [],
});
const failurePayload: Step = {
code: 401,
message: 'Access Denied',
reason: 'Unauthorized',
detail: { failureUrl: 'https://example.com/failure' },
};
setupMockFetch(failurePayload, 401);

const client = await journey({ config: mockConfig });
const result = await client.next(initialStep, {});

expect(result).toBeDefined();
expect(isGenericError(result)).toBe(false);
expect(result).toHaveProperty('type', 'LoginFailure');

if (!isGenericError(result) && result.type === 'LoginFailure') {
expect(result.payload).toEqual(failurePayload);
expect(result.getCode()).toBe(401);
expect(result.getMessage()).toBe('Access Denied');
expect(result.getReason()).toBe('Unauthorized');
}
});

test('redirect_WellknownConfig_StoresStepAndCallsLocationAssign', async () => {
const mockStepPayload: Step = {
callbacks: [
Expand All @@ -204,6 +262,15 @@ describe('journey-client', () => {
};
const step = createJourneyStep(mockStepPayload);
const assignMock = vi.fn();
// Node test environment doesn't provide `window`, so create a minimal shim
// with a real `location` getter so we can keep using vi.spyOn(..., 'get').
(globalThis as unknown as { window?: unknown }).window = {};
Object.defineProperty(window, 'location', {
configurable: true,
get: () => ({
assign: vi.fn(),
}),
});
const locationSpy = vi.spyOn(window, 'location', 'get').mockReturnValue({
...window.location,
assign: assignMock,
Expand Down Expand Up @@ -367,7 +434,7 @@ describe('journey-client', () => {

expect(isGenericError(result)).toBe(true);
if (isGenericError(result)) {
expect(result.error).toBe('no_response_data');
expect(result.error).toBe('request_failed');
expect(result.type).toBe('unknown_error');
}
});
Expand Down
30 changes: 8 additions & 22 deletions packages/journey-client/src/lib/client.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
* Copyright (c) 2025-2026 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.
Expand All @@ -21,7 +21,7 @@ import { createJourneyStore } from './client.store.utils.js';
import { configSlice } from './config.slice.js';
import { journeyApi } from './journey.api.js';
import { createStorage } from '@forgerock/storage';
import { createJourneyObject } from './journey.utils.js';
import { createJourneyObject, resolveJourneyResult } from './journey.utils.js';
import { wellknownApi } from './wellknown.api.js';

import type { JourneyStep } from './step.utils.js';
Expand Down Expand Up @@ -155,32 +155,18 @@ export async function journey<ActionType extends ActionTypes = ActionTypes>({

const self: JourneyClient = {
start: async (options?: StartParam) => {
const { data } = await store.dispatch(journeyApi.endpoints.start.initiate(options));
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);
const { data, error } = await store.dispatch(journeyApi.endpoints.start.initiate(options));
return resolveJourneyResult(data, error);
Copy link
Copy Markdown
Collaborator

@cerebrl cerebrl May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having this new function swallow up createJourneyObject, I'd rather flatten them out. If the main concern here is evaluating the error in a more nuanced manner, then let's do that, but without it absorbing the original function.

For example:

// pseudocode
const { data, error } = await store.dispatch(journeyApi.endpoints.start.initiate(options));
let resolvedError;
if (error) {
  resolvedError = resolveJourneyError(error);
  if (resolveError.catosrophic) {
    return { /* GenericError */ };
  }
}
return createJourneyObject(data || resolvedError):

Copy link
Copy Markdown
Contributor Author

@vatsalparikh vatsalparikh May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Here is how I am thinking about this. journey.utils file should stay pure and not handle RTK errors. RTK error handling should be restricted to journey.api file. If error handling was done in utils file then we're coupling RTK with utils, which introduces coupling between pure utils and API. We should keep createJourneyObject clean by only accepting a Step. We should keep client.store clean by only returning data or error and never doing the actual error handling.

For these reasons, I have moved error handling to journey.api file in a separate function. Let me know if this looks good, thanks!

},

/**
* Submits the current Step payload to the authentication API and retrieves the next JourneyStep in the journey.
*/
next: async (step: JourneyStep, options?: NextOptions) => {
const { data } = await store.dispatch(journeyApi.endpoints.next.initiate({ step, options }));
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);
const { data, error } = await store.dispatch(
journeyApi.endpoints.next.initiate({ step, options }),
);
return resolveJourneyResult(data, error);
},

// TODO: Remove the actual redirect from this method and just return the URL to the caller
Expand Down
Loading
Loading