Skip to content

Commit

Permalink
log 429s to sentry (attempt 2); affects [dynamic endpoint uptimerobot…
Browse files Browse the repository at this point in the history
… weblate opencollective discord github] (badges#9546)

* log to sentry if upstream service responds with 429

* allow services to decide which error(s) to log, default to 429

* don't log 429s from endpoint or dynamic badges

* supress 429s from uptime robot badges

* supress 429s from weblate if not calling default server

* cache opencollective badges for longer

* cache discord badges for longer

* cache github workflow badges for longer
  • Loading branch information
chris48s authored and Lordfirespeed committed Dec 18, 2023
1 parent 019405a commit cd5f392
Show file tree
Hide file tree
Showing 25 changed files with 70 additions and 20 deletions.
4 changes: 4 additions & 0 deletions core/base-service/base-graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class BaseGraphqlService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @param {Function} [attrs.transformJson=data => data] Function which takes the raw json and transforms it before
* further processing. In case of multiple query in a single graphql call and few of them
* throw error, partial data might be used ignoring the error.
Expand All @@ -69,6 +71,7 @@ class BaseGraphqlService extends BaseService {
options = {},
httpErrorMessages = {},
systemErrors = {},
logErrors = [429],
transformJson = data => data,
transformErrors = defaultTransformErrors,
}) {
Expand All @@ -83,6 +86,7 @@ class BaseGraphqlService extends BaseService {
options: mergedOptions,
httpErrors: httpErrorMessages,
systemErrors,
logErrors,
})
const json = transformJson(this._parseJson(buffer))
if (json.errors) {
Expand Down
4 changes: 4 additions & 0 deletions core/base-service/base-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class BaseJsonService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @returns {object} Parsed response
* @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
*/
Expand All @@ -49,6 +51,7 @@ class BaseJsonService extends BaseService {
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
}) {
const mergedOptions = {
...{ headers: { Accept: 'application/json' } },
Expand All @@ -59,6 +62,7 @@ class BaseJsonService extends BaseService {
options: mergedOptions,
httpErrors,
systemErrors,
logErrors,
})
const json = this._parseJson(buffer)
return this.constructor._validate(json, schema)
Expand Down
4 changes: 4 additions & 0 deletions core/base-service/base-svg-scraping.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class BaseSvgScrapingService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @returns {object} Parsed response
* @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
*/
Expand All @@ -73,6 +75,7 @@ class BaseSvgScrapingService extends BaseService {
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
}) {
const logTrace = (...args) => trace.logTrace('fetch', ...args)
const mergedOptions = {
Expand All @@ -84,6 +87,7 @@ class BaseSvgScrapingService extends BaseService {
options: mergedOptions,
httpErrors,
systemErrors,
logErrors,
})
logTrace(emojic.dart, 'Response SVG', buffer)
const data = {
Expand Down
4 changes: 4 additions & 0 deletions core/base-service/base-toml.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class BaseTomlService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @returns {object} Parsed response
* @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
*/
Expand All @@ -42,6 +44,7 @@ class BaseTomlService extends BaseService {
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
}) {
const logTrace = (...args) => trace.logTrace('fetch', ...args)
const mergedOptions = {
Expand All @@ -61,6 +64,7 @@ class BaseTomlService extends BaseService {
options: mergedOptions,
httpErrors,
systemErrors,
logErrors,
})
let parsed
try {
Expand Down
4 changes: 4 additions & 0 deletions core/base-service/base-xml.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class BaseXmlService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @param {object} [attrs.parserOptions={}] Options to pass to fast-xml-parser. See
* [documentation](https://github.com/NaturalIntelligence/fast-xml-parser#xml-to-json)
* @returns {object} Parsed response
Expand All @@ -46,6 +48,7 @@ class BaseXmlService extends BaseService {
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
parserOptions = {},
}) {
const logTrace = (...args) => trace.logTrace('fetch', ...args)
Expand All @@ -58,6 +61,7 @@ class BaseXmlService extends BaseService {
options: mergedOptions,
httpErrors,
systemErrors,
logErrors,
})
const validateResult = XMLValidator.validate(buffer)
if (validateResult !== true) {
Expand Down
4 changes: 4 additions & 0 deletions core/base-service/base-yaml.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class BaseYamlService extends BaseService {
* See {@link https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#errorcodes got error codes}
* for allowed keys
* and {@link module:core/base-service/errors~RuntimeErrorProps} for allowed values
* @param {number[]} [attrs.logErrors=[429]] An array of http error codes
* that will be logged (to sentry, if configured).
* @param {object} [attrs.encoding='utf8'] Character encoding
* @returns {object} Parsed response
* @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
Expand All @@ -43,6 +45,7 @@ class BaseYamlService extends BaseService {
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
encoding = 'utf8',
}) {
const logTrace = (...args) => trace.logTrace('fetch', ...args)
Expand All @@ -60,6 +63,7 @@ class BaseYamlService extends BaseService {
options: mergedOptions,
httpErrors,
systemErrors,
logErrors,
})
let parsed
try {
Expand Down
10 changes: 8 additions & 2 deletions core/base-service/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,13 @@ class BaseService {
this._metricHelper = metricHelper
}

async _request({ url, options = {}, httpErrors = {}, systemErrors = {} }) {
async _request({
url,
options = {},
httpErrors = {},
systemErrors = {},
logErrors = [429],
}) {
const logTrace = (...args) => trace.logTrace('fetch', ...args)
let logUrl = url
const logOptions = Object.assign({}, options)
Expand All @@ -290,7 +296,7 @@ class BaseService {
)
await this._meterResponse(res, buffer)
logTrace(emojic.dart, 'Response status code', res.statusCode)
return checkErrorResponse(httpErrors)({ buffer, res })
return checkErrorResponse(httpErrors, logErrors)({ buffer, res })
}

static enabledMetrics = []
Expand Down
8 changes: 7 additions & 1 deletion core/base-service/check-error-response.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import log from '../server/log.js'
import { NotFound, InvalidResponse, Inaccessible } from './errors.js'

const defaultErrorMessages = {
404: 'not found',
429: 'rate limited by upstream service',
}

export default function checkErrorResponse(httpErrors = {}) {
export default function checkErrorResponse(httpErrors = {}, logErrors = [429]) {
return async function ({ buffer, res }) {
let error
httpErrors = { ...defaultErrorMessages, ...httpErrors }
Expand All @@ -25,6 +26,11 @@ export default function checkErrorResponse(httpErrors = {}) {
error = new InvalidResponse(props)
}
}

if (logErrors.includes(res.statusCode)) {
log.error(new Error(`${res.statusCode} calling ${res.requestUrl.origin}`))
}

if (error) {
error.response = res
error.buffer = buffer
Expand Down
2 changes: 1 addition & 1 deletion core/base-service/check-error-response.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('async error handler', function () {

context('when status is 429', function () {
const buffer = Buffer.from('some stuff')
const res = { statusCode: 429 }
const res = { statusCode: 429, requestUrl: new URL('https://example.com/') }

it('throws InvalidResponse', async function () {
try {
Expand Down
2 changes: 1 addition & 1 deletion services/discord/discord.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default class Discord extends BaseJsonService {
},
}

static _cacheLength = 30
static _cacheLength = 60

static defaultBadgeData = { label: 'chat' }

Expand Down
1 change: 1 addition & 0 deletions services/dynamic/dynamic-json.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export default class DynamicJson extends jsonPath(BaseJsonService) {
schema,
url,
httpErrors,
logErrors: [],
})
}
}
1 change: 1 addition & 0 deletions services/dynamic/dynamic-toml.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export default class DynamicToml extends jsonPath(BaseTomlService) {
schema,
url,
httpErrors,
logErrors: [],
})
}
}
1 change: 1 addition & 0 deletions services/dynamic/dynamic-xml.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export default class DynamicXml extends BaseService {
url,
options: { headers: { Accept: 'application/xml, text/xml' } },
httpErrors,
logErrors: [],
})

const { values: value } = this.transform({
Expand Down
1 change: 1 addition & 0 deletions services/dynamic/dynamic-yaml.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export default class DynamicYaml extends jsonPath(BaseYamlService) {
schema,
url,
httpErrors,
logErrors: [],
})
}
}
1 change: 1 addition & 0 deletions services/endpoint-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ async function fetchEndpointData(
schema: anySchema,
url,
httpErrors,
logErrors: [],
options: { decompress: true },
})
return validateEndpointData(json, {
Expand Down
2 changes: 2 additions & 0 deletions services/github/github-actions-workflow-status.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export default class GithubActionsWorkflowStatus extends BaseSvgScrapingService
},
]

static _cacheLength = 60

static defaultBadgeData = {
label: 'build',
}
Expand Down
2 changes: 1 addition & 1 deletion services/opencollective/opencollective-all.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class OpencollectiveAll extends OpencollectiveBase {
},
}

static _cacheLength = 900
static _cacheLength = 1800

static defaultBadgeData = {
label: 'backers and sponsors',
Expand Down
2 changes: 1 addition & 1 deletion services/opencollective/opencollective-backers.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class OpencollectiveBackers extends OpencollectiveBase {
},
}

static _cacheLength = 900
static _cacheLength = 1800

static defaultBadgeData = {
label: 'backers',
Expand Down
2 changes: 1 addition & 1 deletion services/opencollective/opencollective-sponsors.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class OpencollectiveSponsors extends OpencollectiveBase {
},
}

static _cacheLength = 900
static _cacheLength = 1800

static defaultBadgeData = {
label: 'sponsors',
Expand Down
1 change: 1 addition & 0 deletions services/uptimerobot/uptimerobot-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export default class UptimeRobotBase extends BaseJsonService {
...opts,
},
},
logErrors: [],
})

if (stat === 'fail') {
Expand Down
2 changes: 2 additions & 0 deletions services/weblate/weblate-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Joi from 'joi'
import { BaseJsonService } from '../index.js'
import { optionalUrl } from '../validators.js'

export const defaultServer = 'https://hosted.weblate.org'

export default class WeblateBase extends BaseJsonService {
static queryParamSchema = Joi.object({
server: optionalUrl,
Expand Down
7 changes: 4 additions & 3 deletions services/weblate/weblate-component-license.service.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Joi from 'joi'
import WeblateBase from './weblate-base.js'
import WeblateBase, { defaultServer } from './weblate-base.js'

const schema = Joi.object({
license: Joi.string().required(),
Expand All @@ -21,7 +21,7 @@ export default class WeblateComponentLicense extends WeblateBase {
{
title: 'Weblate component license',
namedParams: { project: 'godot-engine', component: 'godot' },
queryParams: { server: 'https://hosted.weblate.org' },
queryParams: { server: defaultServer },
staticPreview: this.render({ license: 'MIT' }),
keywords: ['i18n', 'translation', 'internationalization'],
},
Expand All @@ -33,14 +33,15 @@ export default class WeblateComponentLicense extends WeblateBase {
return { message: `${license}` }
}

async fetch({ project, component, server = 'https://hosted.weblate.org' }) {
async fetch({ project, component, server = defaultServer }) {
return super.fetch({
schema,
url: `${server}/api/components/${project}/${component}/`,
httpErrors: {
403: 'access denied by remote server',
404: 'component not found',
},
logErrors: server === defaultServer ? [429] : [],
})
}

Expand Down
7 changes: 4 additions & 3 deletions services/weblate/weblate-entities.service.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Joi from 'joi'
import { nonNegativeInteger } from '../validators.js'
import { metric } from '../text-formatters.js'
import WeblateBase from './weblate-base.js'
import WeblateBase, { defaultServer } from './weblate-base.js'

const schema = Joi.object({
count: nonNegativeInteger,
Expand All @@ -20,7 +20,7 @@ export default class WeblateEntities extends WeblateBase {
{
title: 'Weblate entities',
namedParams: { type: 'projects' },
queryParams: { server: 'https://hosted.weblate.org' },
queryParams: { server: defaultServer },
staticPreview: this.render({ type: 'projects', count: 533 }),
keywords: ['i18n', 'internationalization'],
},
Expand All @@ -32,13 +32,14 @@ export default class WeblateEntities extends WeblateBase {
return { label: type, message: metric(count) }
}

async fetch({ type, server = 'https://hosted.weblate.org' }) {
async fetch({ type, server = defaultServer }) {
return super.fetch({
schema,
url: `${server}/api/${type}/`,
httpErrors: {
403: 'access denied by remote server',
},
logErrors: server === defaultServer ? [429] : [],
})
}

Expand Down
Loading

0 comments on commit cd5f392

Please sign in to comment.