From 86c00eb4bd0908c8600cd22df8dbb33f008e87ce Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Fri, 2 Jan 2026 12:22:31 -0700 Subject: [PATCH 1/2] fix: update-storage-client-error-handling --- packages/davinci-client/package.json | 1 + .../davinci-client/src/lib/client.store.ts | 53 +++-- packages/davinci-client/tsconfig.json | 3 + packages/davinci-client/tsconfig.lib.json | 3 + packages/sdk-utilities/package.json | 3 + packages/sdk-utilities/src/index.ts | 1 + .../src/lib/error/error.utils.test.ts | 185 ++++++++++++++++++ .../src/lib/error/error.utils.ts | 24 +++ packages/sdk-utilities/src/lib/error/index.ts | 8 + packages/sdk-utilities/tsconfig.json | 3 + packages/sdk-utilities/tsconfig.lib.json | 6 +- pnpm-lock.yaml | 51 ++--- 12 files changed, 306 insertions(+), 35 deletions(-) create mode 100644 packages/sdk-utilities/src/lib/error/error.utils.test.ts create mode 100644 packages/sdk-utilities/src/lib/error/error.utils.ts create mode 100644 packages/sdk-utilities/src/lib/error/index.ts diff --git a/packages/davinci-client/package.json b/packages/davinci-client/package.json index d58090409..95239d7c4 100644 --- a/packages/davinci-client/package.json +++ b/packages/davinci-client/package.json @@ -27,6 +27,7 @@ "@forgerock/sdk-oidc": "workspace:*", "@forgerock/sdk-request-middleware": "workspace:*", "@forgerock/sdk-types": "workspace:*", + "@forgerock/sdk-utilities": "workspace:*", "@forgerock/storage": "workspace:*", "@reduxjs/toolkit": "catalog:", "effect": "catalog:effect", diff --git a/packages/davinci-client/src/lib/client.store.ts b/packages/davinci-client/src/lib/client.store.ts index a95d0079e..fd29a9d72 100644 --- a/packages/davinci-client/src/lib/client.store.ts +++ b/packages/davinci-client/src/lib/client.store.ts @@ -9,6 +9,7 @@ */ import { CustomLogger, logger as loggerFn, LogLevel } from '@forgerock/sdk-logger'; import { createStorage } from '@forgerock/storage'; +import { isGenericError } from '@forgerock/sdk-utilities'; import { createClientStore, handleUpdateValidateError, RootState } from './client.store.utils.js'; import { nodeSlice } from './node.slice.js'; @@ -126,7 +127,17 @@ export async function davinci({ if (serverSlice && serverSlice.status === 'continue') { return async () => { - await serverInfo.set(serverSlice); + const setResult = await serverInfo.set(serverSlice); + if (isGenericError(setResult)) { + log.error(setResult.message ?? setResult.error); + return { + error: { + message: setResult.message ?? 'Failed to store server info for external IDP flow', + type: 'internal_error', + }, + } as InternalErrorResponse; + } + return; }; } return async () => { @@ -195,27 +206,45 @@ export async function davinci({ }: { continueToken: string; }): Promise => { - try { - const storedServerInfo = (await serverInfo.get()) as ContinueNode['server']; - await store.dispatch( - davinciApi.endpoints.resume.initiate({ continueToken, serverInfo: storedServerInfo }), - ); - await serverInfo.remove(); + const storedServerInfo = await serverInfo.get(); - const node = nodeSlice.selectSlice(store.getState()); + if (storedServerInfo === null) { + log.error('No server info found in storage for resume operation'); + return { + error: { + message: + 'No server info found in storage. Social login needs server info which is saved in local storage. You may have cleared your browser data.', + type: 'state_error', + }, + type: 'internal_error', + }; + } - return node; - } catch { - // logger.error('No url found in collector, social login needs a url in the collector'); + if (isGenericError(storedServerInfo)) { + log.error(storedServerInfo.message ?? storedServerInfo.error); return { error: { message: - 'No url found in storage, social login needs a continue url which is saved in local storage. You may have cleared your browser data', + storedServerInfo.message ?? + 'Failed to retrieve server info from storage for resume operation', type: 'internal_error', }, type: 'internal_error', }; } + + await store.dispatch( + davinciApi.endpoints.resume.initiate({ continueToken, serverInfo: storedServerInfo }), + ); + + const removeResult = await serverInfo.remove(); + if (isGenericError(removeResult)) { + log.warn(removeResult.message ?? 'Failed to remove server info from storage after resume'); + } + + const node = nodeSlice.selectSlice(store.getState()); + + return node; }, /** diff --git a/packages/davinci-client/tsconfig.json b/packages/davinci-client/tsconfig.json index 906cfbf87..6cf14ad15 100644 --- a/packages/davinci-client/tsconfig.json +++ b/packages/davinci-client/tsconfig.json @@ -13,6 +13,9 @@ { "path": "../sdk-effects/storage" }, + { + "path": "../sdk-utilities" + }, { "path": "../sdk-types" }, diff --git a/packages/davinci-client/tsconfig.lib.json b/packages/davinci-client/tsconfig.lib.json index ba359f987..cf394754e 100644 --- a/packages/davinci-client/tsconfig.lib.json +++ b/packages/davinci-client/tsconfig.lib.json @@ -34,6 +34,9 @@ { "path": "../sdk-effects/storage/tsconfig.lib.json" }, + { + "path": "../sdk-utilities/tsconfig.lib.json" + }, { "path": "../sdk-types/tsconfig.lib.json" }, diff --git a/packages/sdk-utilities/package.json b/packages/sdk-utilities/package.json index f2ecce4b1..651498d43 100644 --- a/packages/sdk-utilities/package.json +++ b/packages/sdk-utilities/package.json @@ -36,6 +36,9 @@ "test": "pnpm nx nxTest", "test:watch": "pnpm nx nxTest --watch" }, + "dependencies": { + "@forgerock/sdk-types": "workspace:*" + }, "nx": { "tags": ["scope:sdk-utilities"] } diff --git a/packages/sdk-utilities/src/index.ts b/packages/sdk-utilities/src/index.ts index 265799508..d2ef0ad09 100644 --- a/packages/sdk-utilities/src/index.ts +++ b/packages/sdk-utilities/src/index.ts @@ -7,6 +7,7 @@ * */ +export * from './lib/error/index.js'; export * from './lib/oidc/index.js'; export * from './lib/strings/index.js'; export * from './lib/url/index.js'; diff --git a/packages/sdk-utilities/src/lib/error/error.utils.test.ts b/packages/sdk-utilities/src/lib/error/error.utils.test.ts new file mode 100644 index 000000000..7fd7a0dd8 --- /dev/null +++ b/packages/sdk-utilities/src/lib/error/error.utils.test.ts @@ -0,0 +1,185 @@ +/* + * 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. + */ + +import { describe, it, expect } from 'vitest'; +import { isGenericError } from './error.utils.js'; +import type { GenericError } from '@forgerock/sdk-types'; + +describe('isGenericError', () => { + describe('success cases', () => { + it('isGenericError_ValidGenericErrorWithRequiredFields_ReturnsTrue', () => { + // Arrange + const error: GenericError = { + error: 'storage_error', + type: 'unknown_error', + }; + + // Act + const result = isGenericError(error); + + // Assert + expect(result).toBe(true); + }); + + it('isGenericError_ValidGenericErrorWithAllFields_ReturnsTrue', () => { + // Arrange + const error: GenericError = { + code: 500, + error: 'storage_error', + message: 'Failed to store value', + status: 500, + type: 'unknown_error', + }; + + // Act + const result = isGenericError(error); + + // Assert + expect(result).toBe(true); + }); + + it('isGenericError_ValidGenericErrorWithParseErrorType_ReturnsTrue', () => { + // Arrange + const error: GenericError = { + error: 'Parsing_error', + message: 'Error parsing value from session storage', + type: 'parse_error', + }; + + // Act + const result = isGenericError(error); + + // Assert + expect(result).toBe(true); + }); + }); + + describe('failure cases', () => { + it('isGenericError_NullValue_ReturnsFalse', () => { + // Arrange + const value = null; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_UndefinedValue_ReturnsFalse', () => { + // Arrange + const value = undefined; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_PrimitiveString_ReturnsFalse', () => { + // Arrange + const value = 'error string'; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_PrimitiveNumber_ReturnsFalse', () => { + // Arrange + const value = 500; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_EmptyObject_ReturnsFalse', () => { + // Arrange + const value = {}; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_ObjectMissingErrorProperty_ReturnsFalse', () => { + // Arrange + const value = { + type: 'unknown_error', + message: 'Some error message', + }; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_ObjectMissingTypeProperty_ReturnsFalse', () => { + // Arrange + const value = { + error: 'storage_error', + message: 'Some error message', + }; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_ObjectWithNonStringError_ReturnsFalse', () => { + // Arrange + const value = { + error: 123, + type: 'unknown_error', + }; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_ArrayValue_ReturnsFalse', () => { + // Arrange + const value = ['error', 'type']; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + + it('isGenericError_ValidDataObject_ReturnsFalse', () => { + // Arrange + const value = { + id: '123', + name: 'test', + data: { nested: 'value' }, + }; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + }); +}); diff --git a/packages/sdk-utilities/src/lib/error/error.utils.ts b/packages/sdk-utilities/src/lib/error/error.utils.ts new file mode 100644 index 000000000..de23b09f3 --- /dev/null +++ b/packages/sdk-utilities/src/lib/error/error.utils.ts @@ -0,0 +1,24 @@ +/* + * 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. + */ + +import type { GenericError } from '@forgerock/sdk-types'; + +/** + * Type guard to check if a value is a GenericError. + * + * @param value - The value to check + * @returns True if the value is a GenericError, false otherwise + */ +export function isGenericError(value: unknown): value is GenericError { + return ( + typeof value === 'object' && + value !== null && + 'error' in value && + typeof (value as GenericError).error === 'string' && + 'type' in value + ); +} diff --git a/packages/sdk-utilities/src/lib/error/index.ts b/packages/sdk-utilities/src/lib/error/index.ts new file mode 100644 index 000000000..9fcc91f26 --- /dev/null +++ b/packages/sdk-utilities/src/lib/error/index.ts @@ -0,0 +1,8 @@ +/* + * 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. + */ + +export { isGenericError } from './error.utils.js'; diff --git a/packages/sdk-utilities/tsconfig.json b/packages/sdk-utilities/tsconfig.json index 9d592a77c..5c1aa3dc5 100644 --- a/packages/sdk-utilities/tsconfig.json +++ b/packages/sdk-utilities/tsconfig.json @@ -3,6 +3,9 @@ "files": [], "include": [], "references": [ + { + "path": "../sdk-types" + }, { "path": "./tsconfig.lib.json" }, diff --git a/packages/sdk-utilities/tsconfig.lib.json b/packages/sdk-utilities/tsconfig.lib.json index 4919a8c11..e057aece5 100644 --- a/packages/sdk-utilities/tsconfig.lib.json +++ b/packages/sdk-utilities/tsconfig.lib.json @@ -19,5 +19,9 @@ }, "include": ["src/**/*.ts"], "exclude": ["src/**/*.spec.ts", "src/**/*.test.ts", "src/lib/mock-data/*"], - "references": [] + "references": [ + { + "path": "../sdk-types/tsconfig.lib.json" + } + ] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d8a2d2fa2..e01b284b5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -246,7 +246,7 @@ importers: version: 6.4.1(@types/node@24.9.2)(jiti@2.6.1)(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) vitest: specifier: catalog:vitest - version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) vitest-canvas-mock: specifier: ^0.3.3 version: 0.3.3(vitest@3.2.4) @@ -363,7 +363,7 @@ importers: devDependencies: '@effect/vitest': specifier: catalog:effect - version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) + version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) e2e/oidc-app: dependencies: @@ -398,6 +398,9 @@ importers: '@forgerock/sdk-types': specifier: workspace:* version: link:../sdk-types + '@forgerock/sdk-utilities': + specifier: workspace:* + version: link:../sdk-utilities '@forgerock/storage': specifier: workspace:* version: link:../sdk-effects/storage @@ -413,7 +416,7 @@ importers: devDependencies: vitest: specifier: catalog:vitest - version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) packages/device-client: dependencies: @@ -456,14 +459,14 @@ importers: version: 6.4.1(@types/node@24.9.2)(jiti@2.6.1)(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) vitest-canvas-mock: specifier: ^0.3.3 - version: 0.3.3(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1)) + version: 0.3.3(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1)) devDependencies: '@vitest/coverage-v8': specifier: ^1.2.0 - version: 1.6.1(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1)) + version: 1.6.1(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1)) vitest: specifier: ^1.2.0 - version: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1) + version: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1) packages/oidc-client: dependencies: @@ -494,7 +497,7 @@ importers: devDependencies: '@effect/vitest': specifier: catalog:effect - version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) + version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) msw: specifier: 'catalog:' version: 2.12.1(@types/node@24.9.2)(typescript@5.8.3) @@ -528,7 +531,11 @@ importers: packages/sdk-types: {} - packages/sdk-utilities: {} + packages/sdk-utilities: + dependencies: + '@forgerock/sdk-types': + specifier: workspace:* + version: link:../sdk-types scratchpad: dependencies: @@ -568,14 +575,14 @@ importers: version: 3.19.3 vitest: specifier: catalog:vitest - version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + version: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) devDependencies: '@effect/language-service': specifier: catalog:effect version: 0.35.2 '@effect/vitest': specifier: catalog:effect - version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) + version: 0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1)) packages: @@ -9407,10 +9414,10 @@ snapshots: dependencies: effect: 3.19.3 - '@effect/vitest@0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1))': + '@effect/vitest@0.23.13(effect@3.19.3)(vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1))': dependencies: effect: 3.19.3 - vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) '@effect/workflow@0.8.3(@effect/platform@0.90.10(effect@3.19.3))(@effect/rpc@0.68.4(@effect/platform@0.90.10(effect@3.19.3))(effect@3.19.3))(effect@3.19.3)': dependencies: @@ -10276,7 +10283,7 @@ snapshots: semver: 7.7.3 tsconfig-paths: 4.2.0 vite: 6.4.1(@types/node@24.9.2)(jiti@2.6.1)(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) - vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) transitivePeerDependencies: - '@babel/traverse' - '@swc-node/register' @@ -11258,7 +11265,7 @@ snapshots: lodash: 4.17.21 minimatch: 7.4.6 - '@vitest/coverage-v8@1.6.1(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1))': + '@vitest/coverage-v8@1.6.1(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1))': dependencies: '@ampproject/remapping': 2.3.0 '@bcoe/v8-coverage': 0.2.3 @@ -11273,7 +11280,7 @@ snapshots: std-env: 3.10.0 strip-literal: 2.1.1 test-exclude: 6.0.0 - vitest: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1) + vitest: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1) transitivePeerDependencies: - supports-color @@ -11292,7 +11299,7 @@ snapshots: std-env: 3.10.0 test-exclude: 7.0.1 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) transitivePeerDependencies: - supports-color @@ -11368,7 +11375,7 @@ snapshots: sirv: 3.0.2 tinyglobby: 0.2.15 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) '@vitest/utils@1.6.1': dependencies: @@ -16630,17 +16637,17 @@ snapshots: tsx: 4.20.6 yaml: 2.8.1 - vitest-canvas-mock@0.3.3(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1)): + vitest-canvas-mock@0.3.3(vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1)): dependencies: jest-canvas-mock: 2.5.2 - vitest: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1) + vitest: 1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1) vitest-canvas-mock@0.3.3(vitest@3.2.4): dependencies: jest-canvas-mock: 2.5.2 - vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) + vitest: 3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1) - vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jsdom@27.1.0)(terser@5.44.1): + vitest@1.6.1(@types/node@24.9.2)(@vitest/ui@3.0.4)(jsdom@27.1.0)(terser@5.44.1): dependencies: '@vitest/expect': 1.6.1 '@vitest/runner': 1.6.1 @@ -16676,7 +16683,7 @@ snapshots: - supports-color - terser - vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4(vitest@3.2.4))(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1): + vitest@3.2.4(@types/node@24.9.2)(@vitest/ui@3.0.4)(jiti@2.6.1)(jsdom@27.1.0)(msw@2.12.1(@types/node@24.9.2)(typescript@5.8.3))(terser@5.44.1)(tsx@4.20.6)(yaml@2.8.1): dependencies: '@types/chai': 5.2.3 '@vitest/expect': 3.2.4 From 3c63979f83486e0914b61b6accfd5345e6eff152 Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Mon, 12 Jan 2026 09:17:05 -0700 Subject: [PATCH 2/2] chore: pr-comments --- .changeset/fix-storage-error-handling.md | 11 +++ .../davinci-client/src/lib/client.store.ts | 74 +++++++++++-------- .../storage/src/lib/storage.effects.ts | 24 ++++-- .../src/lib/error/error.utils.test.ts | 14 ++++ .../src/lib/error/error.utils.ts | 3 +- 5 files changed, 90 insertions(+), 36 deletions(-) create mode 100644 .changeset/fix-storage-error-handling.md diff --git a/.changeset/fix-storage-error-handling.md b/.changeset/fix-storage-error-handling.md new file mode 100644 index 000000000..5f31560bd --- /dev/null +++ b/.changeset/fix-storage-error-handling.md @@ -0,0 +1,11 @@ +--- +'@forgerock/davinci-client': patch +'@forgerock/sdk-utilities': patch +'@forgerock/storage': patch +--- + +Fix error handling in storage client and davinci-client + +- Add `isGenericError` type guard to sdk-utilities for runtime error validation +- Fix storage client to properly catch errors from custom storage implementations, honoring the errors-as-values contract +- Improve davinci-client error handling to use explicit error checks instead of try-catch diff --git a/packages/davinci-client/src/lib/client.store.ts b/packages/davinci-client/src/lib/client.store.ts index fd29a9d72..851b17db8 100644 --- a/packages/davinci-client/src/lib/client.store.ts +++ b/packages/davinci-client/src/lib/client.store.ts @@ -206,45 +206,59 @@ export async function davinci({ }: { continueToken: string; }): Promise => { - const storedServerInfo = await serverInfo.get(); + try { + const storedServerInfo = await serverInfo.get(); - if (storedServerInfo === null) { - log.error('No server info found in storage for resume operation'); - return { - error: { - message: - 'No server info found in storage. Social login needs server info which is saved in local storage. You may have cleared your browser data.', - type: 'state_error', - }, - type: 'internal_error', - }; - } + if (storedServerInfo === null) { + log.error('No server info found in storage for resume operation'); + return { + error: { + message: + 'No server info found in storage. Social login needs server info which is saved in local storage. You may have cleared your browser data.', + type: 'state_error', + }, + type: 'internal_error', + }; + } + + if (isGenericError(storedServerInfo)) { + log.error(storedServerInfo.message ?? storedServerInfo.error); + return { + error: { + message: + storedServerInfo.message ?? + 'Failed to retrieve server info from storage for resume operation', + type: 'internal_error', + }, + type: 'internal_error', + }; + } - if (isGenericError(storedServerInfo)) { - log.error(storedServerInfo.message ?? storedServerInfo.error); + await store.dispatch( + davinciApi.endpoints.resume.initiate({ continueToken, serverInfo: storedServerInfo }), + ); + + const removeResult = await serverInfo.remove(); + if (isGenericError(removeResult)) { + log.warn( + removeResult.message ?? 'Failed to remove server info from storage after resume', + ); + } + + const node = nodeSlice.selectSlice(store.getState()); + + return node; + } catch (err) { + const error = err as Error; + log.error(error.message); return { error: { - message: - storedServerInfo.message ?? - 'Failed to retrieve server info from storage for resume operation', + message: error.message ?? 'An unexpected error occurred during resume operation', type: 'internal_error', }, type: 'internal_error', }; } - - await store.dispatch( - davinciApi.endpoints.resume.initiate({ continueToken, serverInfo: storedServerInfo }), - ); - - const removeResult = await serverInfo.remove(); - if (isGenericError(removeResult)) { - log.warn(removeResult.message ?? 'Failed to remove server info from storage after resume'); - } - - const node = nodeSlice.selectSlice(store.getState()); - - return node; }, /** diff --git a/packages/sdk-effects/storage/src/lib/storage.effects.ts b/packages/sdk-effects/storage/src/lib/storage.effects.ts index dd76facdf..64706f07f 100644 --- a/packages/sdk-effects/storage/src/lib/storage.effects.ts +++ b/packages/sdk-effects/storage/src/lib/storage.effects.ts @@ -68,7 +68,13 @@ export function createStorage(config: StorageConfig): StorageClient { if (storeType === 'custom') { - const value = await config.custom.get(key); + let value: Awaited>; + try { + value = await config.custom.get(key); + } catch { + return createStorageError(storeType, 'Retrieving'); + } + if (value === null || (typeof value === 'object' && 'error' in value)) { return value; } @@ -101,8 +107,12 @@ export function createStorage(config: StorageConfig): StorageClient { const valueToStore = JSON.stringify(value); if (storeType === 'custom') { - const value = await config.custom.set(key, valueToStore); - return Promise.resolve(value ?? null); + try { + const result = await config.custom.set(key, valueToStore); + return result ?? null; + } catch { + return createStorageError(storeType, 'Storing'); + } } try { @@ -114,8 +124,12 @@ export function createStorage(config: StorageConfig): StorageClient { if (storeType === 'custom') { - const value = await config.custom.remove(key); - return Promise.resolve(value ?? null); + try { + const result = await config.custom.remove(key); + return result ?? null; + } catch { + return createStorageError(storeType, 'Removing'); + } } try { diff --git a/packages/sdk-utilities/src/lib/error/error.utils.test.ts b/packages/sdk-utilities/src/lib/error/error.utils.test.ts index 7fd7a0dd8..e7a6a37a3 100644 --- a/packages/sdk-utilities/src/lib/error/error.utils.test.ts +++ b/packages/sdk-utilities/src/lib/error/error.utils.test.ts @@ -156,6 +156,20 @@ describe('isGenericError', () => { expect(result).toBe(false); }); + it('isGenericError_ObjectWithNonStringType_ReturnsFalse', () => { + // Arrange + const value = { + error: 'storage_error', + type: 123, + }; + + // Act + const result = isGenericError(value); + + // Assert + expect(result).toBe(false); + }); + it('isGenericError_ArrayValue_ReturnsFalse', () => { // Arrange const value = ['error', 'type']; diff --git a/packages/sdk-utilities/src/lib/error/error.utils.ts b/packages/sdk-utilities/src/lib/error/error.utils.ts index de23b09f3..2db857ab5 100644 --- a/packages/sdk-utilities/src/lib/error/error.utils.ts +++ b/packages/sdk-utilities/src/lib/error/error.utils.ts @@ -19,6 +19,7 @@ export function isGenericError(value: unknown): value is GenericError { value !== null && 'error' in value && typeof (value as GenericError).error === 'string' && - 'type' in value + 'type' in value && + typeof (value as GenericError).type === 'string' ); }