Skip to content

fix: return null when oEmbed response is missing html property - #78

Open
SuparvaCode wants to merge 1 commit into
misskey-dev:masterfrom
SuparvaCode:fix/oembed-missing-html
Open

SuparvaCode wants to merge 1 commit into
misskey-dev:masterfrom
SuparvaCode:fix/oembed-missing-html

Conversation

@SuparvaCode

Copy link
Copy Markdown

Resolves #20

Summary

When an oEmbed endpoint returns a JSON payload without an html property (or where html is undefined), getOEmbedPlayer currently throws a TypeError: Cannot read properties of undefined (reading 'startsWith'). This causes summaly to fail the preview request entirely instead of returning null or falling back to OpenGraph player metadata.

This PR checks that typeof body.html === 'string' before inspecting startsWith / endsWith, and adds an invalid oEmbed test fixture (oembed-no-html.json) to regression test the behavior.

(Apologies for writing in English - my Japanese is limited so I used a live translator to follow the issue description!)

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a9919b66-a39f-412c-8cca-919f7634a156

📥 Commits

Reviewing files that changed from the base of the PR and between c5c60df and b1bcc20.


📒 Files selected for processing (2)
  • src/general.ts
  • test/oembed/invalid/oembed-no-html.json

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

getOEmbedPlayer now checks that body.html is a string before checking iframe boundaries. The invalid-response fixture contains a rich oEmbed response without an HTML field.

Changes

oEmbed HTML validation

Layer / File(s) Summary
Validate oEmbed HTML
src/general.ts, test/oembed/invalid/oembed-no-html.json
getOEmbedPlayer rejects non-string HTML values before checking iframe boundaries. The fixture represents a rich oEmbed response without an HTML field.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium


Merge Risk: ⚪ Minimal · up to b1bcc

Missing-HTML oEmbed responses no longer cause a TypeError, and the caller treats them as absent. The regression fixture is included in generated tests; no actionable merge risk remains from this change.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly describes the main change: return null when the oEmbed response lacks a valid html property.
Description check Passed The description directly explains the TypeError, the type check, and the regression-test fixture added by the pull request.
Linked Issues check Passed Issue #20 requires that an oEmbed response without html does not cause a TypeError. src/general.ts now checks typeof body.html === 'string' before calling startsWith and endsWith, and retu…
Out of Scope Changes check Passed The reviewed changes are limited to the missing-html guard and its regression-test fixture. Both changes directly support Issue #20. No unrelated implementation or test changes are shown.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the HTML string,
No iframe means no broken thing.
A missing field returns null,
The fixture keeps the case in view.
I twitch my nose and hop away!

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

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.

oembedのjsonにhtmlが含まれてないとエラーになる

1 participant