log 429s to sentry (attempt 2); affects [dynamic endpoint uptimerobot weblate opencollective discord github] (#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

core/base-service/base-graphql.js

@@ -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.
@@ -69,6 +71,7 @@ class BaseGraphqlService extends BaseService {
     options = {},
     httpErrorMessages = {},
     systemErrors = {},
+    logErrors = [429],
     transformJson = data => data,
     transformErrors = defaultTransformErrors,
   }) {
@@ -83,6 +86,7 @@ class BaseGraphqlService extends BaseService {
       options: mergedOptions,
       httpErrors: httpErrorMessages,
       systemErrors,
+      logErrors,
     })
     const json = transformJson(this._parseJson(buffer))
     if (json.errors) {

core/base-service/base-json.js

@@ -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
    */
@@ -49,6 +51,7 @@ class BaseJsonService extends BaseService {
     options = {},
     httpErrors = {},
     systemErrors = {},
+    logErrors = [429],
   }) {
     const mergedOptions = {
       ...{ headers: { Accept: 'application/json' } },
@@ -59,6 +62,7 @@ class BaseJsonService extends BaseService {
       options: mergedOptions,
       httpErrors,
       systemErrors,
+      logErrors,
     })
     const json = this._parseJson(buffer)
     return this.constructor._validate(json, schema)

core/base-service/base-svg-scraping.js

@@ -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
    */
@@ -73,6 +75,7 @@ class BaseSvgScrapingService extends BaseService {
     options = {},
     httpErrors = {},
     systemErrors = {},
+    logErrors = [429],
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     const mergedOptions = {
@@ -84,6 +87,7 @@ class BaseSvgScrapingService extends BaseService {
       options: mergedOptions,
       httpErrors,
       systemErrors,
+      logErrors,
     })
     logTrace(emojic.dart, 'Response SVG', buffer)
     const data = {

core/base-service/base-toml.js

@@ -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
    */
@@ -42,6 +44,7 @@ class BaseTomlService extends BaseService {
     options = {},
     httpErrors = {},
     systemErrors = {},
+    logErrors = [429],
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     const mergedOptions = {
@@ -61,6 +64,7 @@ class BaseTomlService extends BaseService {
       options: mergedOptions,
       httpErrors,
       systemErrors,
+      logErrors,
     })
     let parsed
     try {

core/base-service/base-xml.js

@@ -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
@@ -46,6 +48,7 @@ class BaseXmlService extends BaseService {
     options = {},
     httpErrors = {},
     systemErrors = {},
+    logErrors = [429],
     parserOptions = {},
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -58,6 +61,7 @@ class BaseXmlService extends BaseService {
       options: mergedOptions,
       httpErrors,
       systemErrors,
+      logErrors,
     })
     const validateResult = XMLValidator.validate(buffer)
     if (validateResult !== true) {

core/base-service/base-yaml.js

@@ -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
@@ -43,6 +45,7 @@ class BaseYamlService extends BaseService {
     options = {},
     httpErrors = {},
     systemErrors = {},
+    logErrors = [429],
     encoding = 'utf8',
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -60,6 +63,7 @@ class BaseYamlService extends BaseService {
       options: mergedOptions,
       httpErrors,
       systemErrors,
+      logErrors,
     })
     let parsed
     try {

core/base-service/base.js

@@ -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)
@@ -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 = []

core/base-service/check-error-response.js

@@ -1,3 +1,4 @@
+import log from '../server/log.js'
 import { NotFound, InvalidResponse, Inaccessible } from './errors.js'
 
 const defaultErrorMessages = {
@@ -5,7 +6,7 @@ const defaultErrorMessages = {
   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 }
@@ -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

core/base-service/check-error-response.spec.js

@@ -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 {