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

Open
vatsalparikh wants to merge 4 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 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
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.

If we called createJourneyObject with createJourneyObject(data || resolvedError), then we're going back to the initial implementation where I was error handling in journey.api.ts file, which is

ping-javascript-sdk/packages/journey-client/src/lib/journey.api.ts

Line 187 in 13326e0

* If the endpoint returned an HTTP error whose body is an AM Step with a

At the time, you mentioned that "Rather than doing all of this logic here, we could grab error from this destructure: main/packages/journey-client/src/lib/client.store.ts#L174, and pass it to createJourneyObject here: main/packages/journey-client/src/lib/client.store.ts#L183. Finally, just handle the logic here: main/packages/journey-client/src/lib/journey.utils.ts#L28."

And I feel like error handling with resolveJourneyResult handles all errors in one place. If we split like your suggestion, we'll handle some errors in journey.api.ts file, some errors in client.store.ts file and some errors in journey.utils.ts file.

Would be great to discuss further on this over a call to reach a consensus.

},

/**
* 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