Skip to content

Commit

Permalink
Move [StaticBadge] to own service & add test; also affects [gitter] (#…
Browse files Browse the repository at this point in the history
…2284)

This picks up @RedSparr0w's work in #1802.

1. The handler for the static badge is moved into its own service with a synchronous handler. Avoiding an async call may make the static badges slightly faster, though it may be worth profiling this if it turns out we want asynchronous static badges in the future. If it doesn't make a performance difference we could make this handler `async` like the others.
2. Most of the custom static-badge logic is in a BaseStaticBadge base class.
3. Rewrite the static Gitter badge to use BaseStaticBadge.
4. A bit of minor cleanup in related functions.
  • Loading branch information
paulmelnikow authored Nov 17, 2018
1 parent 921adc9 commit 065dd57
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 124 deletions.
1 change: 0 additions & 1 deletion gh-badges/lib/make-badge.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ function assignColor(color = '', colorschemeType = 'colorB') {

const definedColorschemes = require(path.join(__dirname, 'colorscheme.json'))

// Inject the measurer to avoid placing any persistent state in this module.
function makeBadge({
format,
template,
Expand Down
10 changes: 5 additions & 5 deletions lib/request-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ function getBadgeMaxAge(handlerOptions, queryParams) {
? parseInt(process.env.BADGE_MAX_AGE_SECONDS)
: 120
if (handlerOptions.cacheLength) {
// if we've set a more specific cache length for this badge (or category),
// use that instead of env.BADGE_MAX_AGE_SECONDS
maxAge = parseInt(handlerOptions.cacheLength)
// If we've set a more specific cache length for this badge (or category),
// use that instead of env.BADGE_MAX_AGE_SECONDS.
maxAge = handlerOptions.cacheLength
}
if (isInt(queryParams.maxAge) && parseInt(queryParams.maxAge) > maxAge) {
// only allow queryParams.maxAge to override the default
// if it is greater than the default
// Only allow queryParams.maxAge to override the default if it is greater
// than the default.
maxAge = parseInt(queryParams.maxAge)
}
return maxAge
Expand Down
55 changes: 4 additions & 51 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ const log = require('./lib/log')
const makeBadge = require('./gh-badges/lib/make-badge')
const suggest = require('./lib/suggest')
const {
makeColorB,
makeLabel: getLabel,
makeBadgeData: getBadgeData,
setBadgeColor,
} = require('./lib/badge-data')
Expand All @@ -33,7 +31,6 @@ const {
} = require('./lib/request-handler')
const { clearRegularUpdateCache } = require('./lib/regular-update')
const { makeSend } = require('./lib/result-sender')
const { escapeFormat } = require('./lib/path-helpers')

const serverStartTime = new Date(new Date().toGMTString())

Expand Down Expand Up @@ -107,7 +104,10 @@ camp.notfound(/.*/, (query, match, end, request) => {
loadServiceClasses().forEach(serviceClass =>
serviceClass.register(
{ camp, handleRequest: cache, githubApiProvider },
{ handleInternalErrors: config.handleInternalErrors }
{
handleInternalErrors: config.handleInternalErrors,
profiling: config.profiling,
}
)
)

Expand Down Expand Up @@ -227,53 +227,6 @@ camp.route(
})
)

// Any badge.
camp.route(
/^\/(:|badge\/)(([^-]|--)*?)-?(([^-]|--)*)-(([^-]|--)+)\.(svg|png|gif|jpg)$/,
(data, match, end, ask) => {
const subject = escapeFormat(match[2])
const status = escapeFormat(match[4])
const color = escapeFormat(match[6])
const format = match[8]

analytics.noteRequest(data, match)

// Cache management - the badge is constant.
const cacheDuration = (3600 * 24 * 1) | 0 // 1 day.
ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
if (+new Date(ask.req.headers['if-modified-since']) >= +serverStartTime) {
ask.res.statusCode = 304
ask.res.end() // not modified.
return
}
ask.res.setHeader('Last-Modified', serverStartTime.toGMTString())

// Badge creation.
try {
const badgeData = getBadgeData(subject, data)
badgeData.text[0] = getLabel(undefined, { label: subject })
badgeData.text[1] = status
badgeData.colorB = makeColorB(color, data)
badgeData.template = data.style
if (config.profiling.makeBadge) {
console.time('makeBadge total')
}
const svg = makeBadge(badgeData)
if (config.profiling.makeBadge) {
console.timeEnd('makeBadge total')
}
makeSend(format, ask.res, end)(svg)
} catch (e) {
log.error(e.stack)
const svg = makeBadge({
text: ['error', 'bad badge'],
colorscheme: 'red',
})
makeSend(format, ask.res, end)(svg)
}
}
)

// Production cache debugging.
let bitFlip = false
camp.route(/^\/flip\.svg$/, (data, match, end, ask) => {
Expand Down
58 changes: 58 additions & 0 deletions services/base-static.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict'

const makeBadge = require('../gh-badges/lib/make-badge')
const { makeSend } = require('../lib/result-sender')
const analytics = require('../lib/analytics')
const BaseService = require('./base')

const serverStartTime = new Date(new Date().toGMTString())

module.exports = class BaseStaticService extends BaseService {
// Note: Since this is a static service, it is not `async`.
handle(namedParams, queryParams) {
throw new Error(`Handler not implemented for ${this.constructor.name}`)
}

static register({ camp }, serviceConfig) {
camp.route(this._regex, (queryParams, match, end, ask) => {
analytics.noteRequest(queryParams, match)

if (+new Date(ask.req.headers['if-modified-since']) >= +serverStartTime) {
// Send Not Modified.
ask.res.statusCode = 304
ask.res.end()
return
}

const serviceInstance = new this({}, serviceConfig)
const namedParams = this._namedParamsForMatch(match)
let serviceData
try {
// Note: no `await`.
serviceData = serviceInstance.handle(namedParams, queryParams)
} catch (error) {
serviceData = serviceInstance._handleError(error)
}

const badgeData = this._makeBadgeData(queryParams, serviceData)

// The final capture group is the extension.
const format = match.slice(-1)[0]
badgeData.format = format

if (serviceConfig.profiling.makeBadge) {
console.time('makeBadge total')
}
const svg = makeBadge(badgeData)
if (serviceConfig.profiling.makeBadge) {
console.timeEnd('makeBadge total')
}

const cacheDuration = 3600 * 24 * 1 // 1 day.
ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
ask.res.setHeader('Last-Modified', serverStartTime.toGMTString())

makeSend(format, ask.res, end)(svg)
})
}
}
108 changes: 58 additions & 50 deletions services/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const {
const { staticBadgeUrl } = require('../lib/make-badge-url')
const trace = require('./trace')

function coalesce(...candidates) {
return candidates.find(c => typeof c === 'string')
}

class BaseService {
constructor({ sendAndCacheRequest }, { handleInternalErrors }) {
this._requestFetcher = sendAndCacheRequest
Expand Down Expand Up @@ -248,6 +252,52 @@ class BaseService {
return result
}

_handleError(error) {
if (error instanceof NotFound || error instanceof InvalidParameter) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
message: error.prettyMessage,
color: 'red',
}
} else if (
error instanceof InvalidResponse ||
error instanceof Inaccessible ||
error instanceof Deprecated
) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
message: error.prettyMessage,
color: 'lightgray',
}
} else if (this._handleInternalErrors) {
if (
!trace.logTrace(
'unhandledError',
emojic.boom,
'Unhandled internal error',
error
)
) {
// This is where we end up if an unhandled exception is thrown in
// production. Send the error to the logs.
console.log(error)
}
return {
label: 'shields',
message: 'internal error',
color: 'lightgray',
}
} else {
trace.logTrace(
'unhandledError',
emojic.boom,
'Unhandled internal error',
error
)
throw error
}
}

async invokeHandler(namedParams, queryParams) {
trace.logTrace(
'inbound',
Expand All @@ -260,49 +310,7 @@ class BaseService {
try {
return await this.handle(namedParams, queryParams)
} catch (error) {
if (error instanceof NotFound || error instanceof InvalidParameter) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
message: error.prettyMessage,
color: 'red',
}
} else if (
error instanceof InvalidResponse ||
error instanceof Inaccessible ||
error instanceof Deprecated
) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
message: error.prettyMessage,
color: 'lightgray',
}
} else if (this._handleInternalErrors) {
if (
!trace.logTrace(
'unhandledError',
emojic.boom,
'Unhandled internal error',
error
)
) {
// This is where we end up if an unhandled exception is thrown in
// production. Send the error to the logs.
console.log(error)
}
return {
label: 'shields',
message: 'internal error',
color: 'lightgray',
}
} else {
trace.logTrace(
'unhandledError',
emojic.boom,
'Unhandled internal error',
error
)
throw error
}
return this._handleError(error)
}
}

Expand Down Expand Up @@ -333,8 +341,10 @@ class BaseService {

const badgeData = {
text: [
overrideLabel || serviceLabel || defaultLabel || this.category,
serviceMessage || 'n/a',
// Use `coalesce()` to support empty labels and messages, as in the
// static badge.
coalesce(overrideLabel, serviceLabel, defaultLabel, this.category),
coalesce(serviceMessage, 'n/a'),
],
template: style,
logo: makeLogo(style === 'social' ? defaultLogo : undefined, {
Expand All @@ -352,30 +362,28 @@ class BaseService {
}

static register({ camp, handleRequest, githubApiProvider }, serviceConfig) {
const ServiceClass = this // In a static context, "this" is the class.

camp.route(
this._regex,
handleRequest({
queryParams: this.route.queryParams,
handler: async (queryParams, match, sendBadge, request) => {
const namedParams = this._namedParamsForMatch(match)
const serviceInstance = new ServiceClass(
const serviceInstance = new this(
{
sendAndCacheRequest: request.asPromise,
sendAndCacheRequestWithCallbacks: request,
githubApiProvider,
},
serviceConfig
)
const namedParams = this._namedParamsForMatch(match)
const serviceData = await serviceInstance.invokeHandler(
namedParams,
queryParams
)
trace.logTrace('outbound', emojic.shield, 'Service data', serviceData)
const badgeData = this._makeBadgeData(queryParams, serviceData)

// Assumes the final capture group is the extension
// The final capture group is the extension.
const format = match.slice(-1)[0]
sendBadge(format, badgeData)
},
Expand Down
35 changes: 18 additions & 17 deletions services/gitter/gitter.service.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,39 @@
'use strict'

const LegacyService = require('../legacy-service')
const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
const BaseStaticService = require('../base-static')

module.exports = class Gitter extends LegacyService {
module.exports = class Gitter extends BaseStaticService {
static get category() {
return 'chat'
}

static get route() {
return { base: 'gitter/room' }
return {
base: 'gitter/room',
pattern: ':user/:repo',
}
}

static get examples() {
return [
{
title: 'Gitter',
previewUrl: 'nwjs/nw.js',
urlPattern: ':user/:repo',
staticExample: this.render(),
exampleUrl: 'nwjs/nw.js',
},
]
}

static registerLegacyRouteHandler({ camp, cache }) {
camp.route(
/^\/gitter\/room\/([^/]+\/[^/]+)\.(svg|png|gif|jpg|json)$/,
cache((data, match, sendBadge, request) => {
// match[1] is the repo, which is not used.
const format = match[2]
static get defaultBadgeData() {
return { label: 'chat' }
}

static render() {
return { message: 'on gitter', color: 'brightgreen' }
}

const badgeData = getBadgeData('chat', data)
badgeData.text[1] = 'on gitter'
badgeData.colorscheme = 'brightgreen'
sendBadge(format, badgeData)
})
)
handle() {
return this.constructor.render()
}
}
Loading

0 comments on commit 065dd57

Please sign in to comment.