Skip to content

Commit 2816166

Browse files
authored
Convert src/observability to TypeScript (#56391)
1 parent 98af4cf commit 2816166

File tree

9 files changed

+104
-83
lines changed

9 files changed

+104
-83
lines changed

src/frame/start-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import tcpPortUsed from 'tcp-port-used'
44
import dotenv from 'dotenv'
55

66
import { checkNodeVersion } from './lib/check-node-version'
7-
import '../observability/lib/handle-exceptions.js'
7+
import '../observability/lib/handle-exceptions'
88
import createApp from './lib/app'
99
import warmServer from './lib/warm-server'
1010

src/observability/lib/failbot.js

Lines changed: 0 additions & 64 deletions
This file was deleted.

src/observability/lib/failbot.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import got, { type OptionsOfTextResponseBody, type Method } from 'got'
2+
import { Failbot, HTTPBackend } from '@github/failbot'
3+
4+
const HAYSTACK_APP = 'docs'
5+
6+
async function retryingGot(input: RequestInfo | URL, init?: RequestInit): Promise<Response> {
7+
const url = typeof input === 'string' ? input : input.toString()
8+
9+
// Extract body from fetch init for got options
10+
const gotOptions: OptionsOfTextResponseBody = {
11+
method: (init?.method as Method) || 'GET',
12+
body: typeof init?.body === 'string' ? init.body : undefined,
13+
headers: init?.headers as Record<string, string> | undefined,
14+
// With the timeout at 3000 (milliseconds) and the retry.limit
15+
// at 4 (times), the total worst-case is:
16+
// 3000 * 4 + 1000 + 2000 + 3000 + 4000 + 8000 = 30 seconds
17+
timeout: {
18+
response: 3000,
19+
},
20+
retry: {
21+
// This means it will wait...
22+
// 1. 1000ms
23+
// 2. 2000ms
24+
// 3. 4000ms
25+
// 4. 8000ms
26+
// 5. give up!
27+
//
28+
// From the documentation:
29+
//
30+
// Delays between retries counts with function
31+
// 1000 * Math.pow(2, retry - 1) + Math.random() * 100,
32+
// where retry is attempt number (starts from 1).
33+
//
34+
limit: 4,
35+
},
36+
}
37+
38+
const gotResponse = await got(url, gotOptions)
39+
40+
// Convert got response to fetch-compatible Response
41+
return new Response(gotResponse.body, {
42+
status: gotResponse.statusCode,
43+
statusText: gotResponse.statusMessage,
44+
headers: gotResponse.headers as HeadersInit,
45+
})
46+
}
47+
48+
export function report(error: Error, metadata?: Record<string, unknown>) {
49+
// If there's no HAYSTACK_URL set, bail early
50+
if (!process.env.HAYSTACK_URL) {
51+
return
52+
}
53+
54+
const backends = [
55+
new HTTPBackend({
56+
haystackURL: process.env.HAYSTACK_URL,
57+
fetchFn: retryingGot,
58+
}),
59+
]
60+
const failbot = new Failbot({
61+
app: HAYSTACK_APP,
62+
backends,
63+
})
64+
65+
return failbot.report(error, metadata)
66+
}
67+
68+
// Kept for legacy so you can continue to do:
69+
//
70+
// import FailBot from './lib/failbot'
71+
// ...
72+
// FailBot.report(myError)
73+
//
74+
export default {
75+
report,
76+
}

src/observability/lib/handle-exceptions.js renamed to src/observability/lib/handle-exceptions.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import FailBot from './failbot'
22

3-
process.on('uncaughtException', async (err) => {
4-
if (err.code === 'MODULE_NOT_FOUND') {
3+
process.on('uncaughtException', async (err: Error) => {
4+
if ((err as NodeJS.ErrnoException).code === 'MODULE_NOT_FOUND') {
55
console.error('\n\n🔥 Uh oh! It looks you are missing a required npm module.')
66
console.error(
77
'Please run `npm install` to make sure you have all the required dependencies.\n\n',
@@ -17,10 +17,12 @@ process.on('uncaughtException', async (err) => {
1717
}
1818
})
1919

20-
process.on('unhandledRejection', async (err) => {
20+
process.on('unhandledRejection', async (err: Error | unknown) => {
2121
console.error(err)
2222
try {
23-
FailBot.report(err)
23+
// Type guard to ensure we have an Error object for FailBot
24+
const error = err instanceof Error ? err : new Error(String(err))
25+
FailBot.report(error)
2426
} catch (failBotError) {
2527
console.warn('Even sending the unhandledRejection error to FailBot failed!')
2628
console.error(failBotError)

src/observability/lib/statsd.js renamed to src/observability/lib/statsd.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@ const mock = NODE_ENV === 'test' || MODA_PROD_SERVICE_ENV !== 'true'
1414
// MODA_APP_NAME gets set when the deploy target is Moda
1515
const modaApp = MODA_APP_NAME ? `moda_app_name:${MODA_APP_NAME}` : false
1616

17-
export const tags = ['app:docs', modaApp].filter(Boolean)
17+
const tagCandidates = ['app:docs', modaApp]
18+
export const tags: string[] = tagCandidates.filter((tag): tag is string => Boolean(tag))
1819

19-
/**
20-
* @type {import('hot-shots').StatsD}
21-
*/
22-
export default new StatsD({
20+
const statsd = new StatsD({
2321
// When host and port are not set, hot-shots will default to the
2422
// DD_AGENT_HOST and DD_DOGSTATSD_PORT environment variables.
2523
// If undefined, the host will default to 'localhost' and the port
@@ -28,8 +26,10 @@ export default new StatsD({
2826
// For Moda, the host must be set to the Kubernetes node name, which is
2927
// set in KUBE_NODE_HOSTNAME.
3028
host: DD_AGENT_HOST || KUBE_NODE_HOSTNAME,
31-
port: DD_DOGSTATSD_PORT,
29+
port: DD_DOGSTATSD_PORT ? parseInt(DD_DOGSTATSD_PORT, 10) : undefined,
3230
prefix: 'docs.',
3331
mock,
3432
globalTags: tags,
3533
})
34+
35+
export default statsd

src/observability/middleware/catch-middleware-error.js

Lines changed: 0 additions & 3 deletions
This file was deleted.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import type { NextFunction } from 'express'
2+
3+
// Use type assertion to maintain compatibility with existing middleware patterns
4+
// This matches the original JavaScript behavior while providing some type safety
5+
// The assertion is necessary because Express middleware can have various request/response types
6+
export default function catchMiddlewareError(fn: any) {
7+
return (req: any, res: any, next: NextFunction) => Promise.resolve(fn(req, res, next)).catch(next)
8+
}

src/observability/tests/failbot.js renamed to src/observability/tests/failbot.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import nock from 'nock'
44
import FailBot from '../lib/failbot'
55

66
describe('FailBot', () => {
7-
const requestBodiesSent = []
7+
const requestBodiesSent: any[] = []
88

99
beforeEach(() => {
1010
delete process.env.HAYSTACK_URL
@@ -27,7 +27,7 @@ describe('FailBot', () => {
2727

2828
describe('.report', () => {
2929
test('returns early if `HAYSTACK_URL` is not set', async () => {
30-
const result = await FailBot.report()
30+
const result = await FailBot.report(new Error('test'))
3131
expect(result).toBeUndefined()
3232
expect(requestBodiesSent.length).toBe(0)
3333
})
@@ -41,7 +41,9 @@ describe('FailBot', () => {
4141
// But here in the context of vitest, we need to await *now*
4242
// so we can assert that it did make the relevant post requests.
4343
// Once we've done this, we can immediate check what it did.
44-
await Promise.all(await backendPromises)
44+
if (backendPromises) {
45+
await Promise.all(await backendPromises)
46+
}
4547

4648
// It's not interesting or relevant what the `.report()` static
4749
// method returns. All that matters is that it did a POST

src/search/middleware/general-search-middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
This file & middleware is for when a user requests our /search page e.g. 'docs.github.com/search?query=foo'
3-
We make whatever search is in the ?query= parameter and attach it to req.search
4-
req.search is then consumed by the search component in 'src/search/pages/search.tsx'
3+
We make whatever search is in the ?query= parameter and attach it to req.search
4+
req.search is then consumed by the search component in 'src/search/pages/search.tsx'
55
66
When a user directly hits our API e.g. /api/search/v1?query=foo, they will hit the routes in ./search-routes.ts
77
*/

0 commit comments

Comments
 (0)