0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00

🐛 Improved error message output when oembed request fails

refs https://github.com/TryGhost/Ghost/issues/11212

- if a bookmark card fetch is performed (either directly or from fallback) and the page does not have an extractable title, return a more specific error message than "No provider found for supplied URL."
This commit is contained in:
Kevin Ansfield 2020-06-08 15:06:00 +01:00
parent 5aa6a3dbad
commit db68560b11
3 changed files with 103 additions and 5 deletions

View file

@ -45,7 +45,11 @@ async function fetchBookmarkData(url, html) {
metadata
});
}
return Promise.resolve();
return Promise.reject(new errors.ValidationError({
message: i18n.t('errors.api.oembed.insufficientMetadata'),
context: url
}));
}
const findUrlWithProvider = (url) => {
@ -197,6 +201,18 @@ function fetchOembedData(_url) {
});
}
function errorHandler(url) {
return function (err) {
// allow specific validation errors through for better error messages
if (errors.utils.isIgnitionError(err) && err.errorType === 'ValidationError') {
return Promise.reject(err);
}
// default to unknown provider to avoid leaking any app specifics
return unknownProvider(url);
};
}
module.exports = {
docName: 'oembed',
@ -212,7 +228,7 @@ module.exports = {
if (type === 'bookmark') {
return fetchBookmarkData(url)
.catch(() => unknownProvider(url));
.catch(errorHandler(url));
}
return fetchOembedData(url).then((response) => {
@ -225,9 +241,7 @@ module.exports = {
return unknownProvider(url);
}
return response;
}).catch(() => {
return unknownProvider(url);
});
}).catch(errorHandler(url));
}
}
};

View file

@ -438,6 +438,7 @@
},
"oembed": {
"noUrlProvided": "No url provided.",
"insufficientMetadata": "URL contains insufficient metadata for bookmark card.",
"unknownProvider": "No provider found for supplied URL."
},
"userMessages": {

View file

@ -66,6 +66,89 @@ describe('Oembed API', function () {
});
});
describe('type: bookmark', function () {
it('can fetch a bookmark with ?type=bookmark', function (done) {
const pageMock = nock('https://example.com')
.get('/')
.reply(
200,
'<html><head><title>TESTING</title></head><body></body></html>',
{'content-type': 'text/html'}
);
const url = encodeURIComponent('https://example.com');
request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
.set('Origin', config.get('url'))
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
pageMock.isDone().should.be.true();
res.body.type.should.eql('bookmark');
res.body.url.should.eql('https://example.com');
res.body.metadata.title.should.eql('TESTING');
done();
});
});
it('falls back to bookmark without ?type=embed and no oembed metatag', function (done) {
const pageMock = nock('https://example.com')
.get('/')
.times(2) // 1st = oembed metatag check, 2nd = metascraper
.reply(
200,
'<html><head><title>TESTING</title></head><body></body></html>',
{'content-type': 'text/html'}
);
const url = encodeURIComponent('https://example.com');
request.get(localUtils.API.getApiQuery(`oembed/?url=${url}`))
.set('Origin', config.get('url'))
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
pageMock.isDone().should.be.true();
res.body.type.should.eql('bookmark');
res.body.url.should.eql('https://example.com');
res.body.metadata.title.should.eql('TESTING');
done();
});
});
it('errors with useful message when title is unavailable', function (done) {
const pageMock = nock('https://example.com')
.get('/')
.reply(
200,
'<html><head><title></title></head><body></body></html>',
{'content-type': 'text/html'}
);
const url = encodeURIComponent('https://example.com');
request.get(localUtils.API.getApiQuery(`oembed/?type=bookmark&url=${url}`))
.set('Origin', config.get('url'))
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(422)
.end(function (err, res) {
if (err) {
return done(err);
}
pageMock.isDone().should.be.true();
should.exist(res.body.errors);
res.body.errors[0].context.should.match(/insufficient metadata/i);
done();
});
});
});
describe('with unknown provider', function () {
it('fetches url and follows redirects', function (done) {
const redirectMock = nock('http://test.com/')