Skip to content

Commit

Permalink
🐛 Fixed HTTP 500 responses when oembed endpoint receives error
Browse files Browse the repository at this point in the history
fixes TryGhost/Product#4237

- this fixes the fact that we return a HTTP 500 response when the oembed
  library receives an error, such as a 401 or 403
- includes special handling for cases where we want to return a slightly
  different error message
- also adds unit tests for @tryghost/oembed-service package
  • Loading branch information
daniellockyer committed Mar 12, 2024
1 parent 466e3f9 commit 1068b9e
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 15 deletions.
2 changes: 1 addition & 1 deletion ghost/core/test/e2e-api/admin/oembed.test.js
Expand Up @@ -107,7 +107,7 @@ describe('Oembed API', function () {
.set('Origin', config.get('url'))
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(401);
.expect(422);

requestMock.isDone().should.be.true();
should.exist(res.body.errors);
Expand Down
17 changes: 10 additions & 7 deletions ghost/oembed-service/lib/OEmbedService.js
Expand Up @@ -10,6 +10,7 @@ const messages = {
noUrlProvided: 'No url provided.',
insufficientMetadata: 'URL contains insufficient metadata.',
unknownProvider: 'No provider found for supplied URL.',
unableToFetchOembed: 'Unable to fetch requested embed.',
unauthorized: 'URL contains a private resource.'
};

Expand Down Expand Up @@ -101,15 +102,17 @@ class OEmbedService {
try {
return await extract(url);
} catch (err) {
if (err.message === 'Request failed with error code 401') {
throw new errors.UnauthorizedError({
message: messages.unauthorized
});
} else {
throw new errors.InternalServerError({
message: err.message
if (err.message === 'Request failed with error code 401' || err.message === 'Request failed with error code 403') {
throw new errors.ValidationError({
message: tpl(messages.unableToFetchOembed),
context: messages.unauthorized
});
}

throw new errors.ValidationError({
message: tpl(messages.unableToFetchOembed),
context: err.message
});
}
}

Expand Down
7 changes: 0 additions & 7 deletions ghost/oembed-service/test/hello.test.js

This file was deleted.

99 changes: 99 additions & 0 deletions ghost/oembed-service/test/oembed-service.test.js
@@ -0,0 +1,99 @@
const assert = require('assert/strict');
const nock = require('nock');

const OembedService = require('../');

describe('oembed-service', function () {
/** @type {OembedService} */
let oembedService;

before(function () {
oembedService = new OembedService({});

nock.disableNetConnect();
});

afterEach(function () {
nock.cleanAll();
});

describe('known provider', function () {
it('should return data if successful', async function () {
nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(200, {
title: 'Test Title',
author_name: 'Test Author',
author_url: 'https://www.youtube.com/user/testauthor',
html: '<iframe src="https://www.youtube.com/embed/1234"></iframe>'
});

const response = await oembedService.knownProvider('https://www.youtube.com/watch?v=1234');
assert.equal(response.title, 'Test Title');
assert.equal(response.author_name, 'Test Author');
assert.equal(response.author_url, 'https://www.youtube.com/user/testauthor');
assert.equal(response.html, '<iframe src="https://www.youtube.com/embed/1234"></iframe>');
});

it('should return a ValidationError if upstream 401s', async function () {
nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(401);

try {
await oembedService.knownProvider('https://www.youtube.com/watch?v=1234');
} catch (error) {
assert.equal(error.name, 'ValidationError');
assert.equal(error.statusCode, 422);
assert.equal(error.context, 'URL contains a private resource.');
}
});

it('should return a ValidationError if upstream 403s', async function () {
nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(403);

try {
await oembedService.knownProvider('https://www.youtube.com/watch?v=1234');
} catch (error) {
assert.equal(error.name, 'ValidationError');
assert.equal(error.statusCode, 422);
assert.equal(error.context, 'URL contains a private resource.');
}
});

it('should return a ValidationError if upstream 404s', async function () {
nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(404);

try {
await oembedService.knownProvider('https://www.youtube.com/watch?v=1234');
} catch (error) {
assert.equal(error.name, 'ValidationError');
assert.equal(error.statusCode, 422);
assert.equal(error.context, 'Request failed with error code 404');
}
});

it('should return a ValidationError if upstream 500s', async function () {
nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(500);

try {
await oembedService.knownProvider('https://www.youtube.com/watch?v=1234');
} catch (error) {
assert.equal(error.name, 'ValidationError');
assert.equal(error.statusCode, 422);
assert.equal(error.context, 'Request failed with error code 500');
}
});
});
});

0 comments on commit 1068b9e

Please sign in to comment.