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.customExceptions={}] 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 = {},
+    customExceptions = {},
     transformJson = data => data,
     transformErrors = defaultTransformErrors,
   }) {
@@ -75,6 +82,7 @@ class BaseGraphqlService extends BaseService {
       url,
       options: mergedOptions,
       errorMessages: httpErrorMessages,
+      customExceptions,
     })
     const json = transformJson(this._parseJson(buffer))
     if (json.errors) {

core/base-service/base-json.js

@@ -34,10 +34,22 @@ class BaseJsonService 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.customExceptions={}] 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 = {},
+    errorMessages = {},
+    customExceptions = {},
+  }) {
     const mergedOptions = {
       ...{ headers: { Accept: 'application/json' } },
       ...options,
@@ -46,6 +58,7 @@ class BaseJsonService extends BaseService {
       url,
       options: mergedOptions,
       errorMessages,
+      customExceptions,
     })
     const json = this._parseJson(buffer)
     return this.constructor._validate(json, schema)

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

@@ -57,6 +57,12 @@ class BaseSvgScrapingService 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.customExceptions={}] 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
    */
@@ -66,6 +72,7 @@ class BaseSvgScrapingService extends BaseService {
     url,
     options = {},
     errorMessages = {},
+    customExceptions = {},
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     const mergedOptions = {
@@ -76,6 +83,7 @@ class BaseSvgScrapingService extends BaseService {
       url,
       options: mergedOptions,
       errorMessages,
+      customExceptions,
     })
     logTrace(emojic.dart, 'Response SVG', buffer)
     const data = {

core/base-service/base-xml.js

@@ -28,6 +28,12 @@ class BaseXmlService 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.customExceptions={}] 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
@@ -39,6 +45,7 @@ class BaseXmlService extends BaseService {
     url,
     options = {},
     errorMessages = {},
+    customExceptions = {},
     parserOptions = {},
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -50,6 +57,7 @@ class BaseXmlService extends BaseService {
       url,
       options: mergedOptions,
       errorMessages,
+      customExceptions,
     })
     const validateResult = XMLValidator.validate(buffer)
     if (validateResult !== true) {

core/base-service/base-yaml.js

@@ -27,6 +27,12 @@ class BaseYamlService 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.customExceptions={}] 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
@@ -36,6 +42,7 @@ class BaseYamlService extends BaseService {
     url,
     options = {},
     errorMessages = {},
+    customExceptions = {},
     encoding = 'utf8',
   }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
@@ -52,6 +59,7 @@ class BaseYamlService extends BaseService {
       url,
       options: mergedOptions,
       errorMessages,
+      customExceptions,
     })
     let parsed
     try {

core/base-service/base.js

@@ -226,7 +226,12 @@ class BaseService {
     this._metricHelper = metricHelper
   }
 
-  async _request({ url, options = {}, errorMessages = {} }) {
+  async _request({
+    url,
+    options = {},
+    errorMessages = {},
+    customExceptions = {},
+  }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     let logUrl = url
     const logOptions = Object.assign({}, options)
@@ -246,7 +251,11 @@ 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,
+      customExceptions
+    )
     await this._meterResponse(res, buffer)
     logTrace(emojic.dart, 'Response status code', res.statusCode)
     return checkErrorResponse(errorMessages)({ buffer, res })
@@ -328,11 +337,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/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,12 @@ import {
 
 const userAgent = getUserAgent()
 
-async function sendRequest(gotWrapper, url, options) {
+async function sendRequest(
+  gotWrapper,
+  url,
+  options = {},
+  customExceptions = {}
+) {
   const gotOptions = Object.assign({}, options)
   gotOptions.throwHttpErrors = false
   gotOptions.retry = { limit: 0 }
@@ -22,6 +27,12 @@ async function sendRequest(gotWrapper, url, options) {
         underlyingError: new Error('Maximum response size exceeded'),
       })
     }
+    if (err.code in customExceptions) {
+      throw new Inaccessible({
+        ...customExceptions[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.

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

@@ -148,7 +148,7 @@ describe('The request handler', function () {
         expect(headers['cache-control']).to.equal('max-age=900, s-maxage=900')
       })
 
-      it('should let live service data override the default cache headers with longer value', async function () {
+      it('should allow serviceData to override the default cache headers with longer value', async function () {
         camp.route(
           /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/,
           handleRequest(
@@ -168,7 +168,7 @@ describe('The request handler', function () {
         expect(headers['cache-control']).to.equal('max-age=400, s-maxage=400')
       })
 
-      it('should not let live service data override the default cache headers with shorter value', async function () {
+      it('should allow serviceData to override the default cache headers with shorter value', async function () {
         camp.route(
           /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/,
           handleRequest(
@@ -185,7 +185,7 @@ describe('The request handler', function () {
         )
 
         const { headers } = await got(`${baseUrl}/testing/123.json`)
-        expect(headers['cache-control']).to.equal('max-age=300, s-maxage=300')
+        expect(headers['cache-control']).to.equal('max-age=200, s-maxage=200')
       })
 
       it('should set the expires header to current time + cacheSeconds', async function () {

services/endpoint/endpoint.service.js

@@ -178,7 +178,10 @@ export default class Endpoint extends BaseJsonService {
       logoWidth,
       logoPosition,
       style,
-      cacheSeconds,
+      // don't allow the user to set cacheSeconds any shorter than this._cacheLength
+      cacheSeconds: Math.max(
+        ...[this._cacheLength, cacheSeconds].filter(x => x !== undefined)
+      ),
     }
   }