Skip to content

Fixed caller context being dropped from lazy URL compare reports - #29301

Merged
allouis merged 1 commit into
mainfrom
lazy-url-compare-error-details
Jul 14, 2026
Merged

Fixed caller context being dropped from lazy URL compare reports#29301
allouis merged 1 commit into
mainfrom
lazy-url-compare-error-details

Conversation

@allouis

@allouis allouis commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

ref https://linear.app/ghost/issue/HKG-1822

Follow-up to #29190. The caller-context diagnostics added there never reached the logs: @tryghost/errors copies the wrapped error's enumerable properties over the new error, so when the lazy service throws an error carrying its own errorDetails (the thin-resource report), it clobbered the compare context — caller and resourceKeys were silently dropped. Plain test errors carried no errorDetails, which is why the unit tests passed while production reports lost the context.

The report now re-merges both after construction, so log entries carry the thin-resource details and the caller stack + resource shape. This is what's needed to attribute the remaining missing:["tags"] error class (router filters like tag:[newsletter,hash-newsletter] evaluated against resources without a tags array).

🤖 Generated with Claude Code

ref https://linear.app/ghost/issue/HKG-1822

@tryghost/errors copies the wrapped error's enumerable properties over the
new error, so when the lazy service throws with its own errorDetails (the
thin-resource report), it clobbered the compare context — the caller stack
and resource keys never reached the logs. Re-merge the two after
construction so both survive.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff3037f0-d72f-43e4-a894-4989b079d43a

📥 Commits

Reviewing files that changed from the base of the PR and between dac1fef and 7356ded.

📒 Files selected for processing (2)
  • ghost/core/core/server/services/url/url-service-facade.ts
  • ghost/core/test/unit/server/services/url/url-service-facade.test.js

Walkthrough

UrlServiceFacade now preserves errorDetails from errors thrown by the lazy URL service while merging compare context such as method and resource information into the reported InternalServerError. A unit test constructs a typed lazy-service error with existing details, runs compare mode, flushes the asynchronous work, and verifies the logged details contain both sources.

Possibly related PRs

  • TryGhost/Ghost#29190: Updates the same facade and test area for structured compare-mode lazy-error details.

Suggested reviewers: vershwal

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preserving caller context in lazy URL compare reports.
Description check ✅ Passed The description directly explains the bug, the fix, and the impacted logging behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lazy-url-compare-error-details

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

@nx-cloud

nx-cloud Bot commented Jul 14, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 7356ded

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 42s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 36s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 20s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 4s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗
nx run @tryghost/admin:build ✅ Succeeded 4s View ↗
nx run ghost:build:assets ✅ Succeeded 1s View ↗
nx run ghost:build:tsc ✅ Succeeded 7s View ↗
Additional runs (3) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-14 02:16:15 UTC

@allouis
allouis marked this pull request as ready for review July 14, 2026 02:26
@allouis
allouis merged commit 0fa9d88 into main Jul 14, 2026
44 of 45 checks passed
@allouis
allouis deleted the lazy-url-compare-error-details branch July 14, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant