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

Improved error log when Twitter enhanced oembed fails

Browse source 
ref https://linear.app/ghost/issue/ONC-506

- adding `context` with the returned API response makes the logged error much more useful as without it we only log the status code which misses any details for why the failure occurred
This commit is contained in:
Kevin Ansfield 2024-10-31 14:10:31 +00:00
parent 1d429b8b09
commit 7e50a4051f
2 changed files with 72 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
ghost/core/core/server/services/oembed/TwitterOEmbedProvider.js
View file

@ -71,6 +71,14 @@ class TwitterOEmbedProvider {
oembedData.tweet_data = body.data;
oembedData.tweet_data.includes = body.includes;
} catch (err) {
if (err.response?.body) {
try {
const parsed = JSON.parse(err.response.body);
err.context = parsed;
} catch (e) {
err.context = err.response.body;
}
}
logging.error(err);
}
}

83
ghost/core/test/unit/server/services/oembed/twitter-embed.test.js
View file

@ -1,8 +1,11 @@
const assert = require('assert/strict');
const logging = require('@tryghost/logging');
const sinon = require('sinon');
const TwitterOEmbedProvider = require('../../../../../core/server/services/oembed/TwitterOEmbedProvider');
const externalRequest = require('../../../../../core/server/lib/request-external');
const nock = require('nock');
const {mockManager} = require('../../../../utils/e2e-framework');
const {HTTPError} = require('got');
describe('TwitterOEmbedProvider', function () {
before(async function () {
@ -18,6 +21,27 @@ describe('TwitterOEmbedProvider', function () {
mockManager.restore();
});
function nockOembedRequest() {
nock('https://publish.twitter.com')
.get('/oembed')
.query(true)
.reply(200, {
data: {
conversation_id: '1630581157568839683',
public_metrics: {
retweet_count: 6,
reply_count: 1,
like_count: 27
}
},
includes: {
verified: false,
description: 'some description',
location: 'someplace, somewhere'
}
});
}
it('Can support requests for Twitter URLs', async function () {
const provider = new TwitterOEmbedProvider();
const tweetURL = new URL(
@ -48,26 +72,10 @@ describe('TwitterOEmbedProvider', function () {
'https://twitter.com/Ghost/status/1630581157568839683'
);
// not certain why we hit publish.twitter.com
nock('https://publish.twitter.com')
.get('/oembed')
.query(true)
.reply(200, {
data: {
conversation_id: '1630581157568839683',
public_metrics: {
retweet_count: 6,
reply_count: 1,
like_count: 27
}
},
includes: {
verified: false,
description: 'some description',
location: 'someplace, somewhere'
}
});
// oembed-extractor first fetches the main oembed data
nockOembedRequest();
// then we fetch additional data from the Twitter API
nock('https://api.twitter.com')
.get('/2/tweets/1630581157568839683')
.query(true)
@ -93,4 +101,41 @@ describe('TwitterOEmbedProvider', function () {
assert.ok(oembedData.data);
assert.ok(oembedData.includes);
});
it('logs error with context when external request fails', async function () {
const provider = new TwitterOEmbedProvider({
config: {
bearerToken: 'test'
}
});
const tweetURL = new URL(
'https://twitter.com/Ghost/status/1630581157568839683'
);
nockOembedRequest();
nock('https://api.twitter.com')
.get('/2/tweets/1630581157568839683')
.query(true)
.reply(403, {
client_id: '12345',
detail: 'Testing requests are forbidden!',
reason: 'thou-shalt-not-pass'
});
const loggingStub = sinon.stub(logging, 'error');
await provider.getOEmbedData(tweetURL, externalRequest);
sinon.assert.calledOnce(loggingStub);
sinon.assert.calledWithMatch(loggingStub,
sinon.match.instanceOf(HTTPError).and(
sinon.match.has('context', {
client_id: '12345',
detail: 'Testing requests are forbidden!',
reason: 'thou-shalt-not-pass'
})
)
);
});
});