Fix amazon link card#25560
Conversation
WalkthroughAdded metascraper-amazon v5.45.10 to ghost/core/package.json dependencies. Integrated Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📥 CommitsReviewing files that changed from the base of the PR and between e58c16bddb1b73ccb20672da04368b8960275ec3 and efe4c53. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ghost/core/core/server/services/oembed/OEmbedService.js (1)
180-191: Consider broader impact of raising the default fetch timeout to 10sBumping the default timeout from 2s to 10s will slow failure paths for all embeds that go through
fetchPage, not just Amazon, which can affect editor responsiveness and worker utilization if third‑party hosts hang or are very slow. The merge order (...optionslast) still allows callers to override this default, which is good.It may be worth:
- Confirming all call sites that care about latency explicitly set a tighter
options.timeout, and/or- Considering a more targeted longer timeout for known slow domains (e.g. Amazon) instead of globally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between dc7b6c7 and e58c16bddb1b73ccb20672da04368b8960275ec3.
📒 Files selected for processing (2)
ghost/core/core/server/services/oembed/OEmbedService.js(2 hunks)ghost/core/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T16:47:28.150Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 24749
File: ghost/core/core/server/services/members/SingleUseTokenProvider.js:3-5
Timestamp: 2025-08-26T16:47:28.150Z
Learning: When checking for dependencies in Ghost project, ensure to look directly in the specific package.json files rather than relying only on automated searches, as otplib dependency exists at line 204 in ghost/core/package.json version "12.0.1"
Applied to files:
ghost/core/package.json
📚 Learning: 2025-08-12T18:33:15.524Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24658
File: ghost/admin/package.json:3-3
Timestamp: 2025-08-12T18:33:15.524Z
Learning: In Ghost's admin package.json, third-party packages like ember-cli-postcss, ember-exam, and ember-power-select have their own independent versioning schemes that are unrelated to Ghost's version numbers. Version number coincidences between Ghost versions and these packages should not trigger update suggestions.
Applied to files:
ghost/core/package.json
⏰ 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: Cursor Bugbot
🔇 Additional comments (2)
ghost/core/core/server/services/oembed/OEmbedService.js (1)
283-296: Amazon metascraper integration looks consistent, but verify behavior on non‑Amazon pagesAdding
metascraper-amazonat the top of the rules list is consistent with your existing metascraper usage and should enable richer Amazon image extraction while leaving other sites handled by the generic bundles.A couple of things to double‑check:
- That the new plugin doesn’t regress metadata for non‑Amazon URLs (since rule order can influence which values “win”).
- That metascraper’s additional work on Amazon pages doesn’t introduce noticeable delays beyond what the new 10s timeout already allows.
If your new automated test only covers one Amazon URL, consider adding a second from a different Amazon locale or product type to guard against markup variance.
ghost/core/package.json (1)
185-195: metascraper-amazon dependency addition looks consistent with existing metascraper stackThe new
"metascraper-amazon": "5.45.10"entry matches the versions of the othermetascraper-*bundles you’re already using, and it’s correctly added toghost/core/package.jsonalongside the rest of the server‑side metascraper dependencies. No further manifest changes seem necessary here.Based on learnings, locating and updating this dependency in
ghost/core/package.jsonis the right place for this change.
hshiozawa
force-pushed
the
fix-amazon-link-card
branch
from
e58c16b to
efe4c53
Compare
ref TryGhost#22417 ref TryGhost#22590 - 5s balances slow-but-alive pages like Amazon against editor paste latency when a link is dead - regenerated the lockfile for the new metascraper-amazon dependency - covered the amazon ruleset and the fetch timeout with unit tests
9larsons
force-pushed
the
fix-amazon-link-card
branch
from
efe4c53 to
49815a4
Compare
|
Hey @hshiozawa, sorry for the delay here. The changes look good to me and while I think a 2s timeout is reasonable, Amazon pulls are notably slow. I remember looking into this previously. We'll try this out. |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 27s | View ↗ |
nx run ghost:test:ci:e2e |
✅ Succeeded | 12m 31s | View ↗ |
nx build @tryghost/announcement-bar |
✅ Succeeded | 1s | View ↗ |
nx build @tryghost/admin-toolbar |
✅ Succeeded | 1s | View ↗ |
nx build @tryghost/comments-ui |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/signup-form |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/sodo-search |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/activitypub |
✅ Succeeded | 1s | View ↗ |
Additional runs (13) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-12 15:32:16 UTC
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25560 +/- ##
=======================================
Coverage 73.76% 73.76%
=======================================
Files 1541 1541
Lines 132268 132269 +1
Branches 15827 15827
=======================================
+ Hits 97569 97573 +4
+ Misses 33711 33709 -2
+ Partials 988 987 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for the review and merge! |
2 participants
Summary
This PR resolves the issue described below.
#22417
As documented in the linked issue, images are not displayed in Amazon-embedded content.
This behavior also occurs in the latest version (current HEAD) of Ghost.
Root Cause and Fix
I investigated the problem and confirmed that it can be resolved by introducing metascraper-amazon:
#22417 (comment)
Additional Timeout Issue
Large pages like Amazon sometimes cause fetch requests to time out at the default 2-second limit.
I described this problem in detail here, and extending the timeout resolves it:
#22590 (comment)
--
Please check your PR against these items:
We appreciate your contribution! 🙏
Note
Adds
metascraper-amazonto metadata scraping and increases oEmbed page fetch timeout to 10s to improve Amazon link embeds.core/server/services/oembed/OEmbedService.js:fetchPagerequesttimeoutfrom2000to10000ms.metascraper-amazonin metascraper pipeline for bookmark data extraction.core/package.json:metascraper-amazon@5.45.10.Written by Cursor Bugbot for commit efe4c53. This will update automatically on new commits. Configure here.