-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Move [StaticBadge] to own service & add test; also affects [gitter] #2284
Changes from all commits
b2d9016
c1c2ce0
612ac05
5abac51
b85a380
3b3bc36
3613b58
64db5b1
9af7333
58c67f3
ce5ded3
8303a3a
882e42c
d166540
df2feec
c6e0115
8d7b638
a19fcff
7656ba3
964b7d5
bc26948
76c22b2
a4b7241
d574f24
1e2a602
30f0116
906a281
a5f6bce
9d4fe92
aab5906
a4ae95d
704d231
1144f00
2731086
9a36161
d7c5335
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no point in passing this down from |
||
|
||
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 | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently this behavior is not tested. |
||
|
||
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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently this behavior is not tested. |
||
|
||
makeSend(format, ask.res, end)(svg) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -248,6 +252,52 @@ class BaseService { | |
return result | ||
} | ||
|
||
_handleError(error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract into a method so |
||
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', | ||
|
@@ -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) | ||
} | ||
} | ||
|
||
|
@@ -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'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is covered by the static badge tests @RedSparr0w wrote. |
||
], | ||
template: style, | ||
logo: makeLogo(style === 'social' ? defaultLogo : undefined, { | ||
|
@@ -352,30 +362,28 @@ class BaseService { | |
} | ||
|
||
static register({ camp, handleRequest, githubApiProvider }, serviceConfig) { | ||
const ServiceClass = this // In a static context, "this" is the class. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some references use |
||
|
||
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) | ||
}, | ||
|
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() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically unrelated, as in the current implementation the static badge does not call this function.
There's no need for
parseInt
since we're passing numbers, not strings.