Skip to content

Conversation

@ryanbas21
Copy link
Collaborator

@ryanbas21 ryanbas21 commented Dec 10, 2025

This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.

Releases

@forgerock/davinci-client@2.0.0

Major Changes

Patch Changes

  • #510 3c63979 Thanks @ryanbas21! - 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
  • #504 7834cde Thanks @ancheetah! - Improve FIDO module error handling when no options

  • Updated dependencies [3c63979, 9ad4062]:

    • @forgerock/sdk-utilities@2.0.0
    • @forgerock/storage@2.0.0
    • @forgerock/sdk-logger@2.0.0
    • @forgerock/sdk-oidc@2.0.0
    • @forgerock/sdk-request-middleware@2.0.0
    • @forgerock/sdk-types@2.0.0

@forgerock/device-client@2.0.0

Major Changes

@forgerock/journey-client@2.0.0

Major Changes

Patch Changes

  • #500 e99b374 Thanks @ancheetah! - Add support for KBA allowUserDefinedQuestions flag

  • Updated dependencies [3c63979, 9ad4062]:

    • @forgerock/sdk-utilities@2.0.0
    • @forgerock/storage@2.0.0
    • @forgerock/sdk-logger@2.0.0
    • @forgerock/sdk-request-middleware@2.0.0
    • @forgerock/sdk-types@2.0.0

@forgerock/oidc-client@2.0.0

Major Changes

Patch Changes

  • Updated dependencies [3c63979, 9ad4062]:
    • @forgerock/storage@2.0.0
    • @forgerock/iframe-manager@2.0.0
    • @forgerock/sdk-logger@2.0.0
    • @forgerock/sdk-oidc@2.0.0
    • @forgerock/sdk-request-middleware@2.0.0
    • @forgerock/sdk-types@2.0.0

@forgerock/protect@2.0.0

Major Changes

@forgerock/iframe-manager@2.0.0

Major Changes

@forgerock/sdk-logger@2.0.0

Major Changes

@forgerock/sdk-oidc@2.0.0

Major Changes

Patch Changes

  • Updated dependencies [3c63979, 9ad4062]:
    • @forgerock/sdk-utilities@2.0.0
    • @forgerock/sdk-types@2.0.0

@forgerock/sdk-request-middleware@2.0.0

Major Changes

@forgerock/storage@2.0.0

Major Changes

Patch Changes

  • #510 3c63979 Thanks @ryanbas21! - 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
  • Updated dependencies [9ad4062]:

    • @forgerock/sdk-types@2.0.0

@forgerock/sdk-types@2.0.0

Major Changes

@forgerock/sdk-utilities@2.0.0

Major Changes

Patch Changes

  • #510 3c63979 Thanks @ryanbas21! - 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
  • Updated dependencies [9ad4062]:

    • @forgerock/sdk-types@2.0.0

Summary by CodeRabbit

  • New Features
    • Released version 2.0.0 of the ForgeRock SDK with major updates across all core packages, client libraries, and utilities with synchronized dependencies.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

📝 Walkthrough

Walkthrough

Removed several changeset files and bumped many package versions (mostly 1.3.0 → 2.0.0; device-client 0.0.1 → 2.0.0). Added top-level 2.0.0 entries to multiple CHANGELOGs. No source code or public API changes.

Changes

Cohort / File(s) Summary
Changeset removals
.changeset
\.changeset/eleven-baboons-battle.md`, \.changeset/hip-owls-rule.md\, \.changeset/full-bikes-boil.md\, `.changeset/fix-storage-error-handling.md``
Deleted changeset metadata files that documented various patch/major bumps and notes (KBA allowUserDefinedQuestions, multiple package major bumps, davinci-client FIDO note, storage/error-handling fixes).
Clients & product packages
packages/*-client
\packages/davinci-client/package.json`, `packages/davinci-client/CHANGELOG.md`, `packages/journey-client/package.json`, `packages/journey-client/CHANGELOG.md`, `packages/oidc-client/package.json`, `packages/oidc-client/CHANGELOG.md`, `packages/protect/package.json`, `packages/protect/CHANGELOG.md`, `packages/device-client/package.json`, `packages/device-client/CHANGELOG.md``
Version bumps (mostly 1.3.02.0.0; device-client 0.0.12.0.0) and added ## 2.0.0 changelog sections. No code/API changes.
SDK effects packages
packages/sdk-effects/*
\packages/sdk-effects/iframe-manager/package.json`, `packages/sdk-effects/iframe-manager/CHANGELOG.md`, `packages/sdk-effects/logger/package.json`, `packages/sdk-effects/logger/CHANGELOG.md`, `packages/sdk-effects/oidc/package.json`, `packages/sdk-effects/oidc/CHANGELOG.md`, `packages/sdk-effects/sdk-request-middleware/package.json`, `packages/sdk-effects/sdk-request-middleware/CHANGELOG.md`, `packages/sdk-effects/storage/package.json`, `packages/sdk-effects/storage/CHANGELOG.md``
Version bumps to 2.0.0 and added 2.0.0 changelog entries referencing PRs/commits and dependency notes. No functional changes.
SDK foundation packages
packages/sdk-*/
\packages/sdk-types/package.json`, `packages/sdk-types/CHANGELOG.md`, `packages/sdk-utilities/package.json`, `packages/sdk-utilities/CHANGELOG.md``
Version bumps to 2.0.0 and added 2.0.0 changelog sections. Public APIs unchanged.
Manifest summary
packages/.../package.json
All affected package.json files had only the version field updated to 2.0.0 (or device-client to 2.0.0); no other manifest fields altered.

Sequence Diagram(s)

(omitted — changes are metadata/version bumps and deletions; no new multi-component control flow to diagram)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • cerebrl
  • ancheetah

Poem

🐇 I hopped through changelogs with a whisk and cheer,
I nudged the versions up so the release is near.
A few small notes I gently swept away,
Now packages gleam for a brighter day.
🥕 Hooray — a rabbit’s tiny release ballet!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Release PR' is generic and vague, providing no specific information about the actual changes being released. Consider using a more descriptive title such as 'Release: Bump all packages to version 2.0.0' to clearly convey the main change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive and auto-generated by Changesets, containing detailed release notes for 12 packages with version bumps, major changes, and patch changes, but it lacks the JIRA ticket section from the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Contributor

nx-cloud bot commented Dec 10, 2025

View your CI Pipeline Execution ↗ for commit e73b12f

Command Status Duration Result
nx run-many -t build --no-agents ✅ Succeeded <1s View ↗
nx affected -t build lint test e2e-ci ✅ Succeeded 1m 43s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-22 19:14:03 UTC

nx-cloud[bot]

This comment was marked as outdated.

@ryanbas21 ryanbas21 force-pushed the changeset-release/main branch from 8753b12 to d58c175 Compare December 10, 2025 21:46
nx-cloud[bot]

This comment was marked as outdated.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 10, 2025

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/davinci-client@506

@forgerock/device-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/device-client@506

@forgerock/journey-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/journey-client@506

@forgerock/oidc-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/oidc-client@506

@forgerock/protect

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/protect@506

@forgerock/sdk-types

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-types@506

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-utilities@506

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/iframe-manager@506

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-logger@506

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-oidc@506

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-request-middleware@506

@forgerock/storage

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/storage@506

commit: e73b12f

@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.82%. Comparing base (b89ad58) to head (e73b12f).
⚠️ Report is 33 commits behind head on main.

❌ Your project status has failed because the head coverage (18.82%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
+ Coverage   18.79%   18.82%   +0.03%     
==========================================
  Files         140      142       +2     
  Lines       27640    27730      +90     
  Branches      980      991      +11     
==========================================
+ Hits         5195     5221      +26     
- Misses      22445    22509      +64     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

Deployed 2cce169 to https://ForgeRock.github.io/ping-javascript-sdk/pr-506/2cce169e3eb49bac813e20845004ba20b860103e branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

🔻 @forgerock/journey-client - 0.0 KB (-82.5 KB, -100.0%)

➖ No Changes

@forgerock/device-client - 9.2 KB
@forgerock/oidc-client - 23.5 KB
@forgerock/protect - 150.1 KB
@forgerock/sdk-utilities - 8.5 KB
@forgerock/journey-client - 82.5 KB
@forgerock/sdk-types - 8.0 KB
@forgerock/storage - 1.5 KB
@forgerock/sdk-logger - 1.6 KB
@forgerock/iframe-manager - 2.4 KB
@forgerock/sdk-request-middleware - 4.5 KB
@forgerock/sdk-oidc - 2.6 KB
@forgerock/davinci-client - 39.8 KB


13 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated
  • Current Size: Total gzipped size of all files in the package's dist directory
  • Baseline: Comparison against the latest build from the main branch
  • Files included: All build outputs except source maps and TypeScript build cache
  • Exclusions: .map, .tsbuildinfo, and .d.ts.map files

🔄 Updated automatically on each push to this PR

@ryanbas21 ryanbas21 force-pushed the changeset-release/main branch 3 times, most recently from edb5c2a to a86d5eb Compare January 14, 2026 17:30
nx-cloud[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/sdk-effects/logger/CHANGELOG.md`:
- Around line 3-8: Update the 2.0.0 changelog entry in CHANGELOG.md to include
the rationale for the major bump (e.g., rebranding or architectural changes), an
explicit "Breaking changes" subsection listing any API/behavioral changes users
must address, and a short "Migration" subsection with concrete steps or code
examples to help users upgrade; reference the PR (`#502`) and commit (9ad4062) for
details and link to relevant docs or upgrade guides if available.
🧹 Nitpick comments (3)
packages/device-client/CHANGELOG.md (1)

1-7: Consider adding more context about breaking changes.

The changelog entry is correctly formatted but quite minimal for a major version bump (0.0.1 → 2.0.0). Users upgrading would benefit from understanding what breaking changes or API modifications are included in version 2.

Consider enhancing the description with:

  • Key breaking changes introduced
  • Migration guidance or notes
  • Links to detailed migration documentation

This would improve the upgrade experience for package consumers.

packages/sdk-types/CHANGELOG.md (1)

3-8: Consider adding more detail about what changed in v2.0.0.

The changelog entry correctly follows the established format but provides limited information about the major version bump. Users upgrading from v1.x would benefit from understanding what breaking changes (if any) justify the major version, or clarification that this is a coordinated monorepo version synchronization.

📝 Example of a more informative changelog entry
 ## 2.0.0
 
 ### Major Changes
 
-- [`#502`](https://github.com/ForgeRock/ping-javascript-sdk/pull/502) [`9ad4062`](https://github.com/ForgeRock/ping-javascript-sdk/commit/9ad406268dd568d8d6f6447a07b656e317a9da8d) Thanks [`@ryanbas21`](https://github.com/ryanbas21)! - releasing version 2 of the ping javascript sdk
+- [`#502`](https://github.com/ForgeRock/ping-javascript-sdk/pull/502) [`9ad4062`](https://github.com/ForgeRock/ping-javascript-sdk/commit/9ad406268dd568d8d6f6447a07b656e317a9da8d) Thanks [`@ryanbas21`](https://github.com/ryanbas21)! - Release version 2.0.0 of the Ping JavaScript SDK. This is a coordinated major version bump across all packages in the monorepo. No breaking API changes in this package; see individual package changelogs for details on new features and fixes included in this release.
packages/protect/CHANGELOG.md (1)

3-8: Document breaking changes or migration path for 2.0.0.

The changelog entry for version 2.0.0 is generic and doesn't specify what breaking changes exist or how users should migrate from 1.3.0. While the README provides comprehensive usage documentation, it lacks a migration guide. Consider adding breaking change details or a link to migration documentation in the changelog.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efc6acb and a86d5eb.

📒 Files selected for processing (27)
  • .changeset/eleven-baboons-battle.md
  • .changeset/full-bikes-boil.md
  • .changeset/hip-owls-rule.md
  • packages/davinci-client/CHANGELOG.md
  • packages/davinci-client/package.json
  • packages/device-client/CHANGELOG.md
  • packages/device-client/package.json
  • packages/journey-client/CHANGELOG.md
  • packages/journey-client/package.json
  • packages/oidc-client/CHANGELOG.md
  • packages/oidc-client/package.json
  • packages/protect/CHANGELOG.md
  • packages/protect/package.json
  • packages/sdk-effects/iframe-manager/CHANGELOG.md
  • packages/sdk-effects/iframe-manager/package.json
  • packages/sdk-effects/logger/CHANGELOG.md
  • packages/sdk-effects/logger/package.json
  • packages/sdk-effects/oidc/CHANGELOG.md
  • packages/sdk-effects/oidc/package.json
  • packages/sdk-effects/sdk-request-middleware/CHANGELOG.md
  • packages/sdk-effects/sdk-request-middleware/package.json
  • packages/sdk-effects/storage/CHANGELOG.md
  • packages/sdk-effects/storage/package.json
  • packages/sdk-types/CHANGELOG.md
  • packages/sdk-types/package.json
  • packages/sdk-utilities/CHANGELOG.md
  • packages/sdk-utilities/package.json
💤 Files with no reviewable changes (3)
  • .changeset/hip-owls-rule.md
  • .changeset/eleven-baboons-battle.md
  • .changeset/full-bikes-boil.md
🚧 Files skipped from review as they are similar to previous changes (10)
  • packages/journey-client/package.json
  • packages/davinci-client/CHANGELOG.md
  • packages/oidc-client/package.json
  • packages/device-client/package.json
  • packages/sdk-utilities/CHANGELOG.md
  • packages/sdk-types/package.json
  • packages/sdk-effects/iframe-manager/CHANGELOG.md
  • packages/oidc-client/CHANGELOG.md
  • packages/protect/package.json
  • packages/sdk-effects/sdk-request-middleware/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T15:14:00.044Z
Learnt from: ryanbas21
Repo: ForgeRock/ping-javascript-sdk PR: 430
File: packages/journey-client/src/lib/callbacks/name-callback.ts:9-15
Timestamp: 2025-10-22T15:14:00.044Z
Learning: In packages/journey-client, callback classes are internal implementation details not part of the public API. The callbacks barrel (src/lib/callbacks/index.ts) intentionally only exports the base JourneyCallback class. Internal code imports concrete callback classes directly from their individual files (e.g., factory.ts, journey-client.ts).

Applied to files:

  • packages/journey-client/CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: pr
🔇 Additional comments (10)
packages/sdk-effects/logger/package.json (1)

3-3: LGTM!

The version bump from 1.3.0 to 2.0.0 is consistent with the coordinated major release across the monorepo.

packages/sdk-effects/iframe-manager/package.json (1)

3-3: LGTM!

Version bump to 2.0.0 is consistent with the coordinated major release across all packages in this Changesets-managed release PR.

packages/davinci-client/package.json (1)

3-3: LGTM!

The version bump from 1.3.0 to 2.0.0 is correctly formatted and aligns with the coordinated major release across the SDK packages. The workspace:* specifiers for internal dependencies will be resolved to their published versions (2.0.0) at release time.

packages/sdk-effects/oidc/package.json (1)

3-3: LGTM!

Version bump to 2.0.0 is consistent with the coordinated major release across the SDK packages.

packages/sdk-effects/oidc/CHANGELOG.md (1)

3-13: LGTM!

The 2.0.0 changelog entry is well-structured with proper PR/commit references and correctly documents the major release along with the dependency updates to the 2.0.0 versions of @forgerock/sdk-types and @forgerock/sdk-utilities.

packages/sdk-utilities/package.json (1)

3-3: LGTM!

Version bump from 1.3.0 to 2.0.0 is consistent with the coordinated major release across all packages in this monorepo. The package.json structure is valid, and this change aligns with the Changesets release workflow.

packages/sdk-effects/storage/package.json (1)

3-3: LGTM!

Version bump to 2.0.0 is consistent with the major release across the monorepo. The workspace:* reference for @forgerock/sdk-types will be resolved to the correct version at publish time by the Changesets tooling.

packages/sdk-effects/storage/CHANGELOG.md (1)

3-12: LGTM!

The 2.0.0 changelog entry follows the standard Changesets format with proper PR/commit references and correctly notes the dependency update to @forgerock/sdk-types@2.0.0. Version alignment is consistent with package.json.

packages/journey-client/CHANGELOG.md (1)

3-19: LGTM!

The changelog entry for version 2.0.0 is well-formatted and accurate. All PR references, commit hashes, and dependency versions match the PR objectives. The structure is consistent with the existing changelog format.

packages/sdk-effects/sdk-request-middleware/CHANGELOG.md (1)

3-8: LGTM! Changelog entry follows standard release format.

The version 2.0.0 changelog entry is properly formatted with correct references to PR #502 and the associated commit. While the description is generic, this is typical for Changesets-automated release PRs where detailed changes are tracked in individual changeset files.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +3 to +8
## 2.0.0

### Major Changes

- [#502](https://github.com/ForgeRock/ping-javascript-sdk/pull/502) [`9ad4062`](https://github.com/ForgeRock/ping-javascript-sdk/commit/9ad406268dd568d8d6f6447a07b656e317a9da8d) Thanks [@ryanbas21](https://github.com/ryanbas21)! - releasing version 2 of the ping javascript sdk

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add more context to the changelog entry.

The changelog entry for version 2.0.0 is quite generic. Consider documenting:

  • The specific reason for the major version bump (e.g., rebranding, architectural changes, etc.)
  • Whether there are breaking changes users need to address
  • Migration guidance if needed

This helps package consumers understand the impact of upgrading and whether they need to make changes to their code.

🤖 Prompt for AI Agents
In `@packages/sdk-effects/logger/CHANGELOG.md` around lines 3 - 8, Update the
2.0.0 changelog entry in CHANGELOG.md to include the rationale for the major
bump (e.g., rebranding or architectural changes), an explicit "Breaking changes"
subsection listing any API/behavioral changes users must address, and a short
"Migration" subsection with concrete steps or code examples to help users
upgrade; reference the PR (`#502`) and commit (9ad4062) for details and link to
relevant docs or upgrade guides if available.

Copy link
Contributor

@nx-cloud nx-cloud bot left a comment

Choose a reason for hiding this comment

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

Nx Cloud has identified a flaky task in your failed CI:

🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.

Nx Cloud View detailed reasoning in Nx Cloud ↗


🎓 Learn more about Self-Healing CI on nx.dev

@ryanbas21 ryanbas21 force-pushed the changeset-release/main branch from 2c38f40 to f0cccc7 Compare January 20, 2026 21:14
@ryanbas21 ryanbas21 force-pushed the changeset-release/main branch from f0cccc7 to e73b12f Compare January 22, 2026 19:10
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/sdk-effects/storage/CHANGELOG.md`:
- Around line 11-14: The OIDC client’s error handling is inconsistent: update
the revoke, info, and logout flows that call storageClient.get()/set()/remove()
to use the sdk-utilities isGenericError type guard instead of relying on
implicit checks like '!tokens || !("accessToken" in tokens)' or ad-hoc '"error"
in tokens' checks; import isGenericError and replace those conditional branches
in the revoke, info, and logout handlers (and any other storageClient
interaction) to explicitly check isGenericError(result) and handle/log the error
path consistently while preserving the existing success-path logic.
🧹 Nitpick comments (1)
packages/sdk-effects/storage/CHANGELOG.md (1)

5-7: Consider enhancing the major version description.

The major version bump to 2.0.0 is documented with a brief description "releasing version 2 of the ping javascript sdk." For major releases, users typically benefit from more detailed documentation of:

  • What breaking changes are included
  • What migration steps are required
  • What deprecated APIs were removed

If this information is documented elsewhere (e.g., a migration guide), consider adding a reference link here.

Comment on lines +11 to +14
- [#510](https://github.com/ForgeRock/ping-javascript-sdk/pull/510) [`3c63979`](https://github.com/ForgeRock/ping-javascript-sdk/commit/3c63979f83486e0914b61b6accfd5345e6eff152) Thanks [@ryanbas21](https://github.com/ryanbas21)! - 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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for storage client calls in OIDC client
echo "=== Searching for storage client method calls in OIDC client ==="
rg -n 'getItem|setItem|removeItem' packages/sdk-clients/oidc-client/src/ --type=ts || echo "No direct storage calls found"

# Search for error handling patterns
echo -e "\n=== Searching for error handling around storage operations ==="
rg -n -B3 -A3 'getItem|setItem|removeItem' packages/sdk-clients/oidc-client/src/ --type=ts || echo "No pattern found"

# Search for isGenericError usage in OIDC client
echo -e "\n=== Searching for isGenericError usage in OIDC client ==="
rg -n 'isGenericError' packages/sdk-clients/oidc-client/src/ --type=ts || echo "isGenericError not found"

# Check if OIDC client imports from storage client
echo -e "\n=== Searching for storage client imports in OIDC client ==="
rg -n 'from.*storage' packages/sdk-clients/oidc-client/src/ --type=ts || echo "No storage imports found"

Repository: ForgeRock/ping-javascript-sdk

Length of output: 747


🏁 Script executed:

#!/bin/bash
# Find OIDC client directory
echo "=== Looking for OIDC client ==="
find . -type d -name "*oidc*" 2>/dev/null | head -20

# List packages directory
echo -e "\n=== Contents of packages directory ==="
ls -la packages/ | head -30

# Search for files mentioning oidc
echo -e "\n=== Files mentioning OIDC ==="
find . -type f -name "*.ts" -o -name "*.json" | xargs grep -l "oidc-client" 2>/dev/null | head -10

Repository: ForgeRock/ping-javascript-sdk

Length of output: 1236


🏁 Script executed:

#!/bin/bash
# Explore OIDC client structure
echo "=== OIDC client source files ==="
find packages/oidc-client/src -type f -name "*.ts" 2>/dev/null | head -20

# Search for storage client calls
echo -e "\n=== Searching for storage client method calls ==="
rg -n 'getItem|setItem|removeItem' packages/oidc-client/src/ --type=ts -C3 2>/dev/null || echo "No matches"

# Search for error handling patterns
echo -e "\n=== Searching for isGenericError ==="
rg -n 'isGenericError' packages/oidc-client/src/ --type=ts 2>/dev/null || echo "Not found"

# Search for storage imports
echo -e "\n=== Searching for storage-related imports ==="
rg -n 'from.*storage|import.*storage' packages/oidc-client/src/ --type=ts 2>/dev/null || echo "Not found"

# Search for StorageClient usage
echo -e "\n=== Searching for StorageClient ==="
rg -n 'StorageClient|storage\.' packages/oidc-client/src/ --type=ts -C2 2>/dev/null | head -50

Repository: ForgeRock/ping-javascript-sdk

Length of output: 7154


🏁 Script executed:

#!/bin/bash
# Examine client.store.ts for storage client usage
echo "=== client.store.ts: storage client usage ==="
rg -n 'storage\.' packages/oidc-client/src/lib/client.store.ts -C4 --type=ts | head -80

# Examine logout.request.ts for storage client usage
echo -e "\n=== logout.request.ts: storage client usage ==="
rg -n 'storageClient\.' packages/oidc-client/src/lib/logout.request.ts -C4 --type=ts

# Look for try-catch blocks around storage calls
echo -e "\n=== try-catch patterns in client.store.ts ==="
rg -n 'try|catch' packages/oidc-client/src/lib/client.store.ts --type=ts -C2 | head -60

Repository: ForgeRock/ping-javascript-sdk

Length of output: 3196


🏁 Script executed:

#!/bin/bash
# Look for getState implementation which likely calls storage
echo "=== getState implementation ==="
rg -n 'getState' packages/oidc-client/src/lib/client.store.ts -A15 --type=ts | head -100

# Look for actual storage method calls in client.store
echo -e "\n=== Actual storage method calls in client.store ==="
rg -n '\bget\(|\bset\(|\bremove\(' packages/oidc-client/src/lib/client.store.ts --type=ts -B2 -A2

# Check the initialization and storageClient creation
echo -e "\n=== Storage client initialization ==="
rg -n 'createStorage|StorageClient' packages/oidc-client/src/lib/client.store.ts -B2 -A5 --type=ts

Repository: ForgeRock/ping-javascript-sdk

Length of output: 5795


🏁 Script executed:

#!/bin/bash
# Search for isGenericError in entire codebase
echo "=== isGenericError definition and usage ==="
rg -n 'isGenericError' . --type=ts | head -30

# Check sdk-utilities for isGenericError
echo -e "\n=== In sdk-utilities ==="
find packages/sdk-utilities -type f -name "*.ts" | xargs grep -l 'isGenericError' 2>/dev/null

# Check all error handling patterns in logout.request.ts
echo -e "\n=== logout.request.ts full error handling ==="
cat packages/oidc-client/src/lib/logout.request.ts

Repository: ForgeRock/ping-javascript-sdk

Length of output: 6440


OIDC client appears to have been excluded from PR #510's error handling improvements.

The OIDC client uses storageClient.get(), storageClient.set(), and storageClient.remove() but was not updated to use the new isGenericError type guard. Error handling is inconsistent:

  • Line 243-246 (revoke): Explicitly checks if (tokens && 'error' in tokens)
  • Lines 338, 440, 517 (revoke, info, logout): Only check if (!tokens || !('accessToken' in tokens)) — relying on implicit error detection rather than explicit error validation

Consider whether OIDC client error handling should be updated to use isGenericError consistently, or clarify if it's covered by a separate task.

🤖 Prompt for AI Agents
In `@packages/sdk-effects/storage/CHANGELOG.md` around lines 11 - 14, The OIDC
client’s error handling is inconsistent: update the revoke, info, and logout
flows that call storageClient.get()/set()/remove() to use the sdk-utilities
isGenericError type guard instead of relying on implicit checks like '!tokens ||
!("accessToken" in tokens)' or ad-hoc '"error" in tokens' checks; import
isGenericError and replace those conditional branches in the revoke, info, and
logout handlers (and any other storageClient interaction) to explicitly check
isGenericError(result) and handle/log the error path consistently while
preserving the existing success-path logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants