Skip to content
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

Improve error message for YouTube videos that disabled embedding #16048

Closed
1 task done
shafy opened this issue Dec 25, 2022 · 6 comments
Closed
1 task done

Improve error message for YouTube videos that disabled embedding #16048

shafy opened this issue Dec 25, 2022 · 6 comments
Assignees
Labels
bug [triage] something behaving unexpectedly

Comments

@shafy
Copy link

shafy commented Dec 25, 2022

Issue Summary

EDIT: I've figured out that this specific YouTube doesn't allow embedding. Maybe you could improve the error message shown to the user if you get the Unauthorized message (see below) so this causes less confusion. I have also updated the issue title to reflect this. I'm leaving the following text here as was to leave the context.


In general, YouTube embedding works. But this specific returns an error: https://www.youtube.com/watch?v=5f-JlzBuUUU.

CleanShot 2022-12-25 at 20 10 24@2x

The request is the following:

https://cassandradispatch.org/ghost/api/admin/oembed/?url=https://www.youtube.com/watch?v=5f-JlzBuUUU&type=embed

The error response is the following:

{
	"errors": [
		{
			"message": "Internal server error, cannot read oembed.",
			"context": "invalid json response body at https://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3D5f-JlzBuUUU reason: Unexpected token U in JSON at position 0",
			"type": "InternalServerError",
			"details": null,
			"property": null,
			"help": null,
			"code": null,
			"id": "e01223d0-8487-11ed-94c3-8df7a8a46270",
			"ghostErrorCode": null
		}
	]
}

This is becaue the request
https://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3D5f-JlzBuUUU

Returns Unauthorized as the response body.

Steps to Reproduce

  1. Create a new post (or edit an existing one)
  2. Click the "plus" sign to add a new bloc and choose "YouTube"
  3. Paste this URL: https://www.youtube.com/watch?v=5f-JlzBuUUU
  4. See the error

Ghost Version

5.26.3

Node.js Version

16.18.1

How did you install Ghost?

Hetzner, Ubuntu 22.04.1

Database type

MySQL 8

Browser & OS version

No response

Code of Conduct

  • I agree to be friendly and polite to people in this repository
@github-actions github-actions bot added the needs:triage [triage] this needs to be triaged by the Ghost team label Dec 25, 2022
@shafy shafy changed the title Can't embed specific YouTube video Improve error message for YouTube videos that disabled embedding Dec 26, 2022
@ErisDS ErisDS added the bug [triage] something behaving unexpectedly label Jan 2, 2023
@github-actions github-actions bot removed the needs:triage [triage] this needs to be triaged by the Ghost team label Jan 2, 2023
@ErisDS
Copy link
Member

ErisDS commented Jan 2, 2023

Hey there, thank you so much for the detailed bug report.

That does look like something that shouldn't happen! A PR to fix this issue would be very welcome 🙂

@shafy
Copy link
Author

shafy commented Jan 3, 2023

Thanks @ErisDS ! I'm not sure I have a lot of spare time in the next weeks, so if anybody else reading this wants to swoop in, feel free! Otherwise, I'll keep it in mind and might get back to it eventually 😇

@harshmange44
Copy link

@ErisDS @shafy Hey, I would like to work on this one

@cmraible
Copy link
Contributor

cmraible commented Jan 11, 2023

This issue seems to duplicate issue 10624. I traced the problem back to oembed-extractor, which Ghost uses to extract the oembed payload from the provider (in this case Youtube). The extract(url) function doesn't catch the error and passes the payload directly back to Ghost, which is expecting JSON but instead receives Unauthorized and gets confused.

Opened issue 168 with oembed-extractor to see if the author can add in some error handling for this case — issue 39 (same issue) was previously closed because the author couldn't replicate the issue, but I've been able to reliably replicate it with the Youtube URL linked above so hopefully should be able to get it resolved this time.

cmraible added a commit to cmraible/Ghost that referenced this issue Jan 12, 2023
…disabled

fixes TryGhost#16048

- When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed embed.

- This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicity disabled embedding.
cmraible added a commit to cmraible/Ghost that referenced this issue Jan 13, 2023
…disabled

fixes TryGhost#16048

- When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed embed.

- This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicity disabled embedding.
@cmraible
Copy link
Contributor

Status update: PR is ready and approved to merge, but we want to be careful with deployment/rollout.

Last time we updated the oembed-parser package in Pro (~1 year ago), it created a lot of 422 errors. Since we are again trying to upgrade the oembed-parser package again, we want to carefully deploy this to a subset of Pro users, monitor and if successful, then deploy more widely. As such, we are waiting on the right timing to do this merge so that we have a little time to babysit this rollout.

cmraible added a commit to cmraible/Ghost that referenced this issue Mar 7, 2023
refs TryGhost#16048

- When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed emebed.
- This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicitly disabled embedding.
cmraible added a commit to cmraible/Ghost that referenced this issue Mar 10, 2023
refs TryGhost#16048

- When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed emebed.
- This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicitly disabled embedding.
cmraible added a commit to cmraible/Ghost that referenced this issue May 4, 2023
refs TryGhost#16048

- When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed emebed.
- This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicitly disabled embedding.
cmraible added a commit that referenced this issue May 4, 2023
refs #16048

- When attempting to embed a Youtube video that has had embedding
disabled by its owner/author, Ghost displayed a generic error message
that didn't indicate the reason for the failed emebed.
- This change updated the error message when Youtube (or any provider)
returns 401: Unauthorized to indicate that the owner of the resource has
explicitly disabled embedding.
@cmraible
Copy link
Contributor

cmraible commented May 4, 2023

New error message:
CleanShot 2023-05-04 at 16 05 37@2x

Fixed with this commit: 27e4523

@cmraible cmraible closed this as completed May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants