-
Notifications
You must be signed in to change notification settings - Fork 3
Release PR #506
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.
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
base: main
Are you sure you want to change the base?
Release PR #506
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved 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
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
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
View your CI Pipeline Execution ↗ for commit 19f249b
☁️ Nx Cloud last updated this comment at |
8753b12 to
d58c175
Compare
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (14.16%) 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% 14.16% -4.63%
==========================================
Files 140 148 +8
Lines 27640 26055 -1585
Branches 980 999 +19
==========================================
- Hits 5195 3691 -1504
+ Misses 22445 22364 -81 🚀 New features to boost your workflow:
|
|
Deployed 9617bf9 to https://ForgeRock.github.io/ping-javascript-sdk/pr-506/9617bf9d5deb442462abb6fd4936cb430de65c36 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/journey-client - 0.0 KB (-83.3 KB, -100.0%) ➖ No Changes➖ @forgerock/device-client - 9.2 KB 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
edb5c2a to
a86d5eb
Compare
There was a problem hiding this 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
📒 Files selected for processing (27)
.changeset/eleven-baboons-battle.md.changeset/full-bikes-boil.md.changeset/hip-owls-rule.mdpackages/davinci-client/CHANGELOG.mdpackages/davinci-client/package.jsonpackages/device-client/CHANGELOG.mdpackages/device-client/package.jsonpackages/journey-client/CHANGELOG.mdpackages/journey-client/package.jsonpackages/oidc-client/CHANGELOG.mdpackages/oidc-client/package.jsonpackages/protect/CHANGELOG.mdpackages/protect/package.jsonpackages/sdk-effects/iframe-manager/CHANGELOG.mdpackages/sdk-effects/iframe-manager/package.jsonpackages/sdk-effects/logger/CHANGELOG.mdpackages/sdk-effects/logger/package.jsonpackages/sdk-effects/oidc/CHANGELOG.mdpackages/sdk-effects/oidc/package.jsonpackages/sdk-effects/sdk-request-middleware/CHANGELOG.mdpackages/sdk-effects/sdk-request-middleware/package.jsonpackages/sdk-effects/storage/CHANGELOG.mdpackages/sdk-effects/storage/package.jsonpackages/sdk-types/CHANGELOG.mdpackages/sdk-types/package.jsonpackages/sdk-utilities/CHANGELOG.mdpackages/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-typesand@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-typeswill 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 withpackage.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
#502and 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.
| ## 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 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
f0cccc7 to
e73b12f
Compare
There was a problem hiding this 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.
| - [#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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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 -10Repository: 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 -50Repository: 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 -60Repository: 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=tsRepository: 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.tsRepository: 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.
e73b12f to
396b0d8
Compare
There was a problem hiding this 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-types/CHANGELOG.md`:
- Around line 5-8: Update the "Major Changes" entry that currently reads
"releasing version 2 of the ping javascript sdk" (the entry referencing PR `#502`
and commit 9ad4062) to explicitly list the breaking API changes and required
migration steps, and add a dedicated migration guide file (e.g., MIGRATION_GUIDE
or MIGRATION.md) linked from the CHANGELOG.md; in the changelog entry and
migration guide include a concise bullet list of: specific removed/renamed
functions or classes and their replacements, signature or behavioral changes
callers must update, configuration or build changes, and any removed deprecated
features, plus short before/after code snippets and links to PR `#502` and commit
9ad4062 for details.
| ### 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 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for migration guides or upgrade documentation
fd -t f -e md | xargs rg -l "migration|breaking changes|upgrade.*2\.0|v2.*breaking" -i
# Check if there's a top-level migration guide
fd -t f "MIGRATION|UPGRADING|BREAKING" -i
# Also check README files for upgrade information
fd -t f "README" -e md | xargs rg -l "2\.0|version 2|upgrade" -iRepository: ForgeRock/ping-javascript-sdk
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Search for migration or breaking changes documentation
# First, check if there are any markdown files mentioning migration or breaking changes
echo "=== Searching for migration/breaking changes docs ==="
rg -i "migration|breaking.change|upgrade.*guide" --type md -l || echo "No migration docs found"
# Check all CHANGELOG files for more context
echo -e "\n=== Checking other CHANGELOG files ==="
fd -t f "CHANGELOG.md" --exec echo "File: {}" \; --exec head -50 {} \;
# Look for any MIGRATION or UPGRADING files
echo -e "\n=== Looking for migration guides ==="
fd -i -t f "migration|upgrading" || echo "No migration guide files found"Repository: ForgeRock/ping-javascript-sdk
Length of output: 35097
🏁 Script executed:
#!/bin/bash
# Search for PR `#502` details in the repository
echo "=== Searching for PR `#502` context ==="
rg "#502" --type md -B 2 -A 2
# Check if there's a .changeset directory with details
echo -e "\n=== Checking for changeset files ==="
if [ -d ".changeset" ]; then
ls -la .changeset/
echo -e "\n=== Recent changeset contents ==="
find .changeset -name "*.md" -type f | head -5 | while read file; do
echo "File: $file"
cat "$file"
echo "---"
done
else
echo "No .changeset directory found"
fiRepository: ForgeRock/ping-javascript-sdk
Length of output: 7130
Clarify breaking changes in the major version description.
The major changes description "releasing version 2 of the ping javascript sdk" doesn't specify what breaking changes users should expect when upgrading from 1.x to 2.0.0. No migration guide or breaking changes documentation exists in the repository. For major version bumps, users need clear documentation of:
- Specific API changes that break backward compatibility
- Required code changes for upgrading from 1.x
- Any deprecated features that were removed
Consider expanding this changelog entry or creating a migration guide to help users plan their upgrade.
🤖 Prompt for AI Agents
In `@packages/sdk-types/CHANGELOG.md` around lines 5 - 8, Update the "Major
Changes" entry that currently reads "releasing version 2 of the ping javascript
sdk" (the entry referencing PR `#502` and commit 9ad4062) to explicitly list the
breaking API changes and required migration steps, and add a dedicated migration
guide file (e.g., MIGRATION_GUIDE or MIGRATION.md) linked from the CHANGELOG.md;
in the changelog entry and migration guide include a concise bullet list of:
specific removed/renamed functions or classes and their replacements, signature
or behavioral changes callers must update, configuration or build changes, and
any removed deprecated features, plus short before/after code snippets and links
to PR `#502` and commit 9ad4062 for details.
adb15f2 to
641ae1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the e2e test failure by updating the journey-app test application to handle the new GenericError return type from journey-client v2.0.0. The breaking changes introduced in this release PR mean that journey client methods (start, next, terminate) now return GenericError objects instead of undefined for error cases, and our test application now properly checks for these errors using the 'error' in result pattern as documented in the migration guide.
Tip
✅ We verified this fix by re-running @forgerock/journey-suites:e2e-ci--src/protect.test.ts.
Suggested Fix changes
diff --git a/e2e/journey-app/main.ts b/e2e/journey-app/main.ts
index d5868c1e..cd09bb54 100644
--- a/e2e/journey-app/main.ts
+++ b/e2e/journey-app/main.ts
@@ -59,6 +59,15 @@ if (searchParams.get('middleware') === 'true') {
let step = await journeyClient.start({ journey: journeyName });
+ // Handle GenericError from journey client
+ if (step && 'error' in step) {
+ console.error('Journey start failed with error:', step.message);
+ if (errorEl) {
+ errorEl.innerHTML = `<pre id="errorMessage">Journey start failed: ${step.message}</pre>`;
+ }
+ return;
+ }
+
function renderComplete() {
if (step?.type !== 'LoginSuccess') {
throw new Error('Expected step to be defined and of type LoginSuccess');
@@ -77,12 +86,30 @@ if (searchParams.get('middleware') === 'true') {
const loginBtn = document.getElementById('logoutButton') as HTMLButtonElement;
loginBtn.addEventListener('click', async () => {
- await journeyClient.terminate();
+ const terminateResult = await journeyClient.terminate();
+
+ // Handle GenericError from terminate
+ if (terminateResult && 'error' in terminateResult) {
+ console.error('Logout failed with error:', terminateResult.message);
+ if (errorEl) {
+ errorEl.innerHTML = `<pre id="errorMessage">Logout failed: ${terminateResult.message}</pre>`;
+ }
+ return;
+ }
console.log('Logout successful');
step = await journeyClient.start({ journey: journeyName });
+ // Handle GenericError from start after logout
+ if (step && 'error' in step) {
+ console.error('Journey start after logout failed with error:', step.message);
+ if (errorEl) {
+ errorEl.innerHTML = `<pre id="errorMessage">Journey restart failed: ${step.message}</pre>`;
+ }
+ return;
+ }
+
renderForm();
});
}
@@ -148,6 +175,15 @@ if (searchParams.get('middleware') === 'true') {
query: { noSession: searchParams.get('no-session') || 'false' },
});
+ // Handle GenericError from next
+ if (step && 'error' in step) {
+ console.error('Journey next failed with error:', step.message);
+ if (errorEl) {
+ errorEl.innerHTML = `<pre id="errorMessage">Journey failed: ${step.message}</pre>`;
+ }
+ return;
+ }
+
/**
* Recursively render the form with the new state
*/
Or Apply changes locally with:
npx nx-cloud apply-locally 0vV3-ipUa
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
641ae1a to
19f249b
Compare
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
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
#510
3c63979Thanks @ryanbas21! - Fix error handling in storage client and davinci-clientisGenericErrortype guard to sdk-utilities for runtime error validation#504
7834cdeThanks @ancheetah! - Improve FIDO module error handling when no options#526
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.Updated dependencies [
3c63979,ad81c13,9ad4062,5a9ea40]:@forgerock/device-client@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkMinor Changes
bca228eThanks @ancheetah! - Expose return types for clientsPatch Changes
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/journey-client@2.0.0
Major Changes
#524
4c17ba5Thanks @ryanbas21! - BREAKING CHANGE: Journey client methods now returnGenericErrorinstead ofundefinedfor error cases.What Changed
The
start,next,resume, andterminatemethods now return aGenericErrorobject instead ofundefinedwhen encountering unknown step types or error conditions. This aligns the journey client with the DaVinci client's error handling patterns.Return Type Changes
startJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | undefinedJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericErrornextJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | undefinedJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericErrorresumeJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | undefinedJourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericErrorterminatevoidvoid | GenericErrorMigration Guide
Before:
#502
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkMinor Changes
bca228eThanks @ancheetah! - Expose return types for clientsPatch Changes
#500
e99b374Thanks @ancheetah! - Add support for KBAallowUserDefinedQuestionsflagUpdated dependencies [
3c63979,ad81c13,9ad4062,5a9ea40]:@forgerock/oidc-client@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkMinor Changes
bca228eThanks @ancheetah! - Expose return types for clientsPatch Changes
#526
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.Updated dependencies [
3c63979,ad81c13,9ad4062,5a9ea40]:@forgerock/protect@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/iframe-manager@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/sdk-logger@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
#528
ad81c13Thanks @ancheetah! - Fixes double logging when using custom logger. Custom logger should override default console logs.#526
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/sdk-oidc@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
3c63979,9ad4062,5a9ea40]:@forgerock/sdk-request-middleware@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/storage@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
#510
3c63979Thanks @ryanbas21! - Fix error handling in storage client and davinci-clientisGenericErrortype guard to sdk-utilities for runtime error validation#526
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.Updated dependencies [
9ad4062,5a9ea40]:@forgerock/sdk-types@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.@forgerock/sdk-utilities@2.0.0
Major Changes
9ad4062Thanks @ryanbas21! - releasing version 2 of the ping javascript sdkPatch Changes
#510
3c63979Thanks @ryanbas21! - Fix error handling in storage client and davinci-clientisGenericErrortype guard to sdk-utilities for runtime error validation#526
5a9ea40Thanks @ancheetah! - Update READMES. Fix types and comments.Updated dependencies [
9ad4062,5a9ea40]: