Implement a pattern for dealing with upstream APIs which are slow on the first hit; affects [endpoint]

core/base-service/base-graphql.js

@@ -44,6 +44,12 @@ class BaseGraphqlService extends BaseService {
    *    and custom error messages e.g: `{ 404: 'package not found' }`.
    *    This can be used to extend or override the
    *    [default](https://github.com/badges/shields/blob/master/core/base-service/check-error-response.js#L5)
+   * @param {object} [attrs.systemErrors={}] Key-value map of got network exception codes
+   *    and an object of params to pass when we construct an Inaccessible exception object
+   *    e.g: `{ ECONNRESET: { prettyMessage: 'connection reset' } }`.
+   *    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 {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.
@@ -62,6 +68,7 @@ class BaseGraphqlService extends BaseService {
     variables = {},
     options = {},
     httpErrorMessages = {},
+    systemErrors = {},
     transformJson = data => data,
     transformErrors = defaultTransformErrors,
   }) {
@@ -74,7 +81,8 @@ class BaseGraphqlService extends BaseService {
     const { buffer } = await this._request({
       url,
       options: mergedOptions,
-      errorMessages: httpErrorMessages,
+      httpErrors: httpErrorMessages,
+      systemErrors,
     })
     const json = transformJson(this._parseJson(buffer))
     if (json.errors) {

core/base-service/base-json.js

@@ -30,22 +30,35 @@ class BaseJsonService extends BaseService {
    * @param {string} attrs.url URL to request
    * @param {object} [attrs.options={}] Options to pass to got. See
    *    [documentation](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md)
-   * @param {object} [attrs.errorMessages={}] Key-value map of status codes
+   * @param {object} [attrs.httpErrors={}] Key-value map of status codes
    *    and custom error messages e.g: `{ 404: 'package not found' }`.
    *    This can be used to extend or override the
    *    [default](https://github.com/badges/shields/blob/master/core/base-service/check-error-response.js#L5)
+   * @param {object} [attrs.systemErrors={}] Key-value map of got network exception codes
+   *    and an object of params to pass when we construct an Inaccessible exception object
+   *    e.g: `{ ECONNRESET: { prettyMessage: 'connection reset' } }`.
+   *    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
    * @returns {object} Parsed response
    * @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
    */
-  async _requestJson({ schema, url, options = {}, errorMessages = {} }) {
+  async _requestJson({
+    schema,
+    url,
+    options = {},
+    httpErrors = {},
+    systemErrors = {},
+  }) {
     const mergedOptions = {
       ...{ headers: { Accept: 'application/json' } },
       ...options,
     }
     const { buffer } = await this._request({
       url,
       options: mergedOptions,
-      errorMessages,
+      httpErrors,
+      systemErrors,
     })
     const json = this._parseJson(buffer)
     return this.constructor._validate(json, schema)

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

@@ -53,10 +53,16 @@ class BaseSvgScrapingService extends BaseService {
    * @param {string} attrs.url URL to request
    * @param {object} [attrs.options={}] Options to pass to got. See
    *    [documentation](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md)
-   * @param {object} [attrs.errorMessages={}] Key-value map of status codes
+   * @param {object} [attrs.httpErrors={}] Key-value map of status codes
    *    and custom error messages e.g: `{ 404: 'package not found' }`.
    *    This can be used to extend or override the
    *    [default](https://github.com/badges/shields/blob/master/core/base-service/check-error-response.js#L5)
+   * @param {object} [attrs.systemErrors={}] Key-value map of got network exception codes
+   *    and an object of params to pass when we construct an Inaccessible exception object
+   *    e.g: `{ ECONNRESET: { prettyMessage: 'connection reset' } }`.
+   *    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
    * @returns {object} Parsed response
    * @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
    */
@@ -65,7 +71,8 @@ class BaseSvgScrapingService extends BaseService {
     valueMatcher,
     url,
     options = {},
-    errorMessages = {},
+    httpErrors = {},
+    systemErrors = {},
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     const mergedOptions = {
@@ -75,7 +82,8 @@ class BaseSvgScrapingService extends BaseService {
     const { buffer } = await this._request({
       url,
       options: mergedOptions,
-      errorMessages,
+      httpErrors,
+      systemErrors,
     })
     logTrace(emojic.dart, 'Response SVG', buffer)
     const data = {

core/base-service/base-xml.js

@@ -24,10 +24,16 @@ class BaseXmlService extends BaseService {
    * @param {string} attrs.url URL to request
    * @param {object} [attrs.options={}] Options to pass to got. See
    *    [documentation](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md)
-   * @param {object} [attrs.errorMessages={}] Key-value map of status codes
+   * @param {object} [attrs.httpErrors={}] Key-value map of status codes
    *    and custom error messages e.g: `{ 404: 'package not found' }`.
    *    This can be used to extend or override the
    *    [default](https://github.com/badges/shields/blob/master/core/base-service/check-error-response.js#L5)
+   * @param {object} [attrs.systemErrors={}] Key-value map of got network exception codes
+   *    and an object of params to pass when we construct an Inaccessible exception object
+   *    e.g: `{ ECONNRESET: { prettyMessage: 'connection reset' } }`.
+   *    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 {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
@@ -38,7 +44,8 @@ class BaseXmlService extends BaseService {
     schema,
     url,
     options = {},
-    errorMessages = {},
+    httpErrors = {},
+    systemErrors = {},
     parserOptions = {},
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -49,7 +56,8 @@ class BaseXmlService extends BaseService {
     const { buffer } = await this._request({
       url,
       options: mergedOptions,
-      errorMessages,
+      httpErrors,
+      systemErrors,
     })
     const validateResult = XMLValidator.validate(buffer)
     if (validateResult !== true) {

core/base-service/base-yaml.js

@@ -23,10 +23,16 @@ class BaseYamlService extends BaseService {
    * @param {string} attrs.url URL to request
    * @param {object} [attrs.options={}] Options to pass to got. See
    *    [documentation](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md)
-   * @param {object} [attrs.errorMessages={}] Key-value map of status codes
+   * @param {object} [attrs.httpErrors={}] Key-value map of status codes
    *    and custom error messages e.g: `{ 404: 'package not found' }`.
    *    This can be used to extend or override the
    *    [default](https://github.com/badges/shields/blob/master/core/base-service/check-error-response.js#L5)
+   * @param {object} [attrs.systemErrors={}] Key-value map of got network exception codes
+   *    and an object of params to pass when we construct an Inaccessible exception object
+   *    e.g: `{ ECONNRESET: { prettyMessage: 'connection reset' } }`.
+   *    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 {object} [attrs.encoding='utf8'] Character encoding
    * @returns {object} Parsed response
    * @see https://github.com/sindresorhus/got/blob/main/documentation/2-options.md
@@ -35,7 +41,8 @@ class BaseYamlService extends BaseService {
     schema,
     url,
     options = {},
-    errorMessages = {},
+    httpErrors = {},
+    systemErrors = {},
     encoding = 'utf8',
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -51,7 +58,8 @@ class BaseYamlService extends BaseService {
     const { buffer } = await this._request({
       url,
       options: mergedOptions,
-      errorMessages,
+      httpErrors,
+      systemErrors,
     })
     let parsed
     try {

core/base-service/base.js

@@ -226,7 +226,7 @@ class BaseService {
     this._metricHelper = metricHelper
   }
 
-  async _request({ url, options = {}, errorMessages = {} }) {
+  async _request({ url, options = {}, httpErrors = {}, systemErrors = {} }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     let logUrl = url
     const logOptions = Object.assign({}, options)
@@ -246,10 +246,14 @@ class BaseService {
       'Request',
       `${logUrl}\n${JSON.stringify(logOptions, null, 2)}`
     )
-    const { res, buffer } = await this._requestFetcher(url, options)
+    const { res, buffer } = await this._requestFetcher(
+      url,
+      options,
+      systemErrors
+    )
     await this._meterResponse(res, buffer)
     logTrace(emojic.dart, 'Response status code', res.statusCode)
-    return checkErrorResponse(errorMessages)({ buffer, res })
+    return checkErrorResponse(httpErrors)({ buffer, res })
   }
 
   static enabledMetrics = []
@@ -328,11 +332,15 @@ class BaseService {
       error instanceof Deprecated
     ) {
       trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
-      return {
+      const serviceData = {
         isError: true,
         message: error.prettyMessage,
         color: 'lightgray',
       }
+      if (error.cacheSeconds !== undefined) {
+        serviceData.cacheSeconds = error.cacheSeconds
+      }
+      return serviceData
     } else if (this._handleInternalErrors) {
       if (
         !trace.logTrace(

core/base-service/cache-headers.js

@@ -39,14 +39,14 @@ function coalesceCacheLength({
   assert(defaultCacheLengthSeconds !== undefined)
 
   const cacheLength = coalesce(
+    serviceOverrideCacheLengthSeconds,
     serviceDefaultCacheLengthSeconds,
     defaultCacheLengthSeconds
   )
 
   // Overrides can apply _more_ caching, but not less. Query param overriding
   // can request more overriding than service override, but not less.
   const candidateOverrides = [
-    serviceOverrideCacheLengthSeconds,
     overrideCacheLengthFromQueryParams(queryParams),
   ].filter(x => x !== undefined)
 

core/base-service/cache-headers.spec.js

@@ -74,12 +74,12 @@ describe('Cache header functions', function () {
         serviceDefaultCacheLengthSeconds: 900,
         serviceOverrideCacheLengthSeconds: 400,
         queryParams: {},
-      }).expect(900)
+      }).expect(400)
       given({
         cacheHeaderConfig,
         serviceOverrideCacheLengthSeconds: 400,
         queryParams: {},
-      }).expect(777)
+      }).expect(400)
       given({
         cacheHeaderConfig,
         serviceOverrideCacheLengthSeconds: 900,

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

@@ -5,19 +5,19 @@ const defaultErrorMessages = {
   429: 'rate limited by upstream service',
 }
 
-export default function checkErrorResponse(errorMessages = {}) {
+export default function checkErrorResponse(httpErrors = {}) {
   return async function ({ buffer, res }) {
     let error
-    errorMessages = { ...defaultErrorMessages, ...errorMessages }
+    httpErrors = { ...defaultErrorMessages, ...httpErrors }
     if (res.statusCode === 404) {
-      error = new NotFound({ prettyMessage: errorMessages[404] })
+      error = new NotFound({ prettyMessage: httpErrors[404] })
     } else if (res.statusCode !== 200) {
       const underlying = Error(
         `Got status code ${res.statusCode} (expected 200)`
       )
       const props = { underlyingError: underlying }
-      if (errorMessages[res.statusCode] !== undefined) {
-        props.prettyMessage = errorMessages[res.statusCode]
+      if (httpErrors[res.statusCode] !== undefined) {
+        props.prettyMessage = httpErrors[res.statusCode]
       }
       if (res.statusCode >= 500) {
         error = new Inaccessible(props)

core/base-service/errors.js

@@ -42,6 +42,7 @@ class ShieldsRuntimeError extends Error {
     if (props.underlyingError) {
       this.stack = props.underlyingError.stack
     }
+    this.cacheSeconds = props.cacheSeconds
   }
 }
 
@@ -206,6 +207,9 @@ class Deprecated extends ShieldsRuntimeError {
  * @property {string} prettyMessage User-facing error message to override the
  * value of `defaultPrettyMessage()`. This is the text that will appear on the
  * badge when we catch and render the exception (Optional)
+ * @property {number} cacheSeconds Length of time to cache this error response
+ * for. Defaults to the cacheLength of the service class throwing the error
+ * (Optional)
  */
 
 export {

core/base-service/got.js

@@ -7,7 +7,7 @@ import {
 
 const userAgent = getUserAgent()
 
-async function sendRequest(gotWrapper, url, options) {
+async function sendRequest(gotWrapper, url, options = {}, systemErrors = {}) {
   const gotOptions = Object.assign({}, options)
   gotOptions.throwHttpErrors = false
   gotOptions.retry = { limit: 0 }
@@ -22,6 +22,12 @@ async function sendRequest(gotWrapper, url, options) {
         underlyingError: new Error('Maximum response size exceeded'),
       })
     }
+    if (err.code in systemErrors) {
+      throw new Inaccessible({
+        ...systemErrors[err.code],
+        underlyingError: err,
+      })
+    }
     throw new Inaccessible({ underlyingError: err })
   }
 }

core/base-service/got.spec.js

@@ -45,6 +45,36 @@ describe('got wrapper', function () {
     )
   })
 
+  it('should throw a custom error if provided', async function () {
+    const sendRequest = _fetchFactory(1024)
+    return (
+      expect(
+        sendRequest(
+          'https://www.google.com/foo/bar',
+          { timeout: { request: 1 } },
+          {
+            ETIMEDOUT: {
+              prettyMessage: 'Oh no! A terrible thing has happened',
+              cacheSeconds: 10,
+            },
+          }
+        )
+      )
+        .to.be.rejectedWith(
+          Inaccessible,
+          "Inaccessible: Timeout awaiting 'request' for 1ms"
+        )
+        // eslint-disable-next-line promise/prefer-await-to-then
+        .then(error => {
+          expect(error).to.have.property(
+            'prettyMessage',
+            'Oh no! A terrible thing has happened'
+          )
+          expect(error).to.have.property('cacheSeconds', 10)
+        })
+    )
+  })
+
   it('should pass a custom user agent header', async function () {
     nock('https://www.google.com', {
       reqheaders: {

core/base-service/legacy-request-handler.js

@@ -73,11 +73,9 @@ function handleRequest(cacheHeaderConfig, handlerOptions) {
     // `defaultCacheLengthSeconds` can be overridden by
     // `serviceDefaultCacheLengthSeconds` (either by category or on a badge-
     // by-badge basis). Then in turn that can be overridden by
-    // `serviceOverrideCacheLengthSeconds` (which we expect to be used only in
-    // the dynamic badge) but only if `serviceOverrideCacheLengthSeconds` is
-    // longer than `serviceDefaultCacheLengthSeconds` and then the `cacheSeconds`
-    // query param can also override both of those but again only if `cacheSeconds`
-    // is longer.
+    // `serviceOverrideCacheLengthSeconds`.
+    // Then the `cacheSeconds` query param can also override both of those
+    // but only if `cacheSeconds` is longer.
     //
     // When the legacy services have been rewritten, all the code in here
     // will go away, which should achieve this goal in a simpler way.