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

log 429s to sentry (attempt 2); affects [dynamic endpoint uptimerobot weblate opencollective discord github] #9546

Merged
merged 8 commits into from
Dec 4, 2023
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 procesing. 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 @@ -45,7 +45,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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets see if this gets us consistently under the limits. If not, I'll raise an issue with OC and see if they'd be willing to grant us a higher limit.


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: [],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #9155 for details.

The rate limits we're hitting here are on the user-supplied tokens. There's not really anything we can do about this.

})

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
Loading