Skip to content

Commit

Permalink
send errors to Failbot in SSR rendering (github#30153)
Browse files Browse the repository at this point in the history
* send errors to failbot in SSR rendering

* wip

* progress

* tidying up

* no point awaiting something that doens't return a promise
  • Loading branch information
peterbe authored Aug 22, 2022
1 parent be09c09 commit 8eb7898
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 15 deletions.
7 changes: 5 additions & 2 deletions lib/failbot.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ async function retryingGot(url, args) {
)
}

export async function report(error, metadata) {
export function report(error, metadata) {
// If there's no HAYSTACK_URL set, bail early
if (!process.env.HAYSTACK_URL) return
if (!process.env.HAYSTACK_URL) {
return
}

const backends = [
new HTTPBackend({
Expand All @@ -47,6 +49,7 @@ export async function report(error, metadata) {
app: HAYSTACK_APP,
backends,
})

return failbot.report(error, metadata)
}

Expand Down
4 changes: 2 additions & 2 deletions lib/handle-exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ process.on('uncaughtException', async (err) => {

console.error(err)
try {
await FailBot.report(err)
FailBot.report(err)
} catch (failBotError) {
console.warn('Even sending the uncaughtException error to FailBot failed!')
console.error(failBotError)
Expand All @@ -20,7 +20,7 @@ process.on('uncaughtException', async (err) => {
process.on('unhandledRejection', async (err) => {
console.error(err)
try {
await FailBot.report(err)
FailBot.report(err)
} catch (failBotError) {
console.warn('Even sending the unhandledRejection error to FailBot failed!')
console.error(failBotError)
Expand Down
8 changes: 8 additions & 0 deletions middleware/render-page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { get } from 'lodash-es'

import FailBot from '../lib/failbot.js'
import patterns from '../lib/patterns.js'
import getMiniTocItems from '../lib/get-mini-toc-items.js'
import Page from '../lib/page.js'
Expand Down Expand Up @@ -40,6 +41,13 @@ async function buildMiniTocItems(req) {

export default async function renderPage(req, res, next) {
const { context } = req

// This is a contextualizing the request so that when this `req` is
// ultimately passed into the `Error.getInitialProps` function,
// which NextJS executes at runtime on errors, so that we can
// from there send the error to Failbot.
req.FailBot = FailBot

const { page } = context
const path = req.pagePath || req.path
browserCacheControl(res)
Expand Down
4 changes: 3 additions & 1 deletion next.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import fs from 'fs'
import frontmatter from 'gray-matter'
import path from 'path'

import frontmatter from 'gray-matter'

const homepage = path.posix.join(process.cwd(), 'content/index.md')
const { data } = frontmatter(fs.readFileSync(homepage, 'utf8'))
const productIds = data.children
Expand Down
5 changes: 0 additions & 5 deletions pages/500.tsx

This file was deleted.

104 changes: 99 additions & 5 deletions pages/_error.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,104 @@
import { NextPage } from 'next'
import { GenericError } from 'components/GenericError'
import type { NextPageContext } from 'next'

type Props = {}
import { GenericError } from 'components/GenericError'

const ErrorPage: NextPage<Props> = () => {
function Error() {
return <GenericError />
}

export default ErrorPage
Error.getInitialProps = async (ctx: NextPageContext) => {
// If this getInitialProps() is called in client-side rendering,
// you won't have a `.res` object. It's only present when it's
// rendered Node (SSR). That's our clue to know that, we should
// send this error to Failbot.
// In client-side, it's undefined. In server, it's a ServerResponse object.
const { err, req, res } = ctx
let statusCode = 500
if (res?.statusCode) {
statusCode = res.statusCode
}

// 'err' will by falsy if it's a 404
// But note, at the time of writing this comment, we have a dedicated
// `pages/404.tsx` which takes care of 404 messages.
if (err && res && req) {
// This is a (necessary) hack!
// You can't import `../lib/failbot.js` here in this
// file because it gets imported by webpack to be used in the
// client-side JS bundle. It *could* be solved by overriding
// the webpack configuration in our `next.config.js` but this is prone
// to be fragile since ignoring code can be hard to get right
// and the more we override there, the harder it will become to
// upgrade NextJS in the future because of moving parts.
// So the solution is to essentially do what the contextualizers
// do which mutate the Express request object by attaching
// callables to it. This way it's only ever present in SSR executed
// code and doesn't need any custom webpack configuration.
const expressRequest = req as any
const FailBot = expressRequest.FailBot
if (FailBot) {
try {
// An inclusion-list of headers we're OK with sending because
// they don't contain an PII.
const OK_HEADER_KEYS = ['user-agent', 'referer', 'accept-encoding', 'accept-language']
const reported = FailBot.report(err, {
path: req.url,
request: JSON.stringify(
{
method: expressRequest.method,
query: expressRequest.query,
language: expressRequest.language,
},
undefined,
2
),
headers: JSON.stringify(
Object.fromEntries(
Object.entries(req.headers).filter(([k]) => OK_HEADER_KEYS.includes(k))
),
undefined,
2
),
})

// Within FailBot.report() (which is our wrapper for Failbot in
// the `@github/failbot` package), it might exit only because
// it has no configured backends to send to. I.e. it returns undefined.
// Otherwise, it should return `Array<Promise<Response | void>>`.
if (!reported) {
console.warn(
'The FailBot.report() returned undefined which means the error was NOT sent to Failbot.'
)
} else if (
Array.isArray(reported) &&
reported.length &&
reported.every((thing) => thing instanceof Promise)
) {
// Make sure we await the promises even though we don't care
// about the results. This makes the code cleaner rather than
// letting the eventloop take care of it which could result
// in cryptic error messages if the await does fail for some
// reason.
try {
await Promise.all(reported)
} catch (error) {
console.warn('Unable to await reported FailBot errors', error)
}
}
} catch (error) {
// This does not necessarily mean FailBot failed to send. It's
// most likely that we failed to *send to* FailBot before it
// even has a chance to use the network. This is because
// `FailBot.report` returns an array of Promises which themselves
// could go wrong, but that's a story for another try/catch.
// Basically, this catch it just to avoid other errors that
// might prevent the pretty error page to render at all.
console.warn('Failed to send error to FailBot.', error)
}
}
}

return { statusCode, message: err?.message }
}

export default Error

0 comments on commit 8eb7898

Please sign in to comment.