Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/github-apps/lib/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@
"2022-11-28"
]
},
"sha": "9f10fd5d6e100db6f5f36d56967e9304eaa3b1b0"
"sha": "04fd6c592fc546217404b07e0b0e581fb00a963a"
}
27 changes: 27 additions & 0 deletions src/observability/lib/should-log-exception.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
export type ErrorWithCode = Error & {
code: string
statusCode?: number
status?: string
}

export function shouldLogException(error: ErrorWithCode) {
const IGNORED_ERRORS = [
// Client connected aborted
'ECONNRESET',
]

if (IGNORED_ERRORS.includes(error.code)) {
return false
}

// "TypeError: terminated" is thrown by Node.js's undici fetch implementation
// when a connection is aborted by the remote server. This is a transient
// network error (similar to ECONNRESET) that occurs during proxied requests
// to archived enterprise pages and shouldn't be reported to Sentry.
if (error.name === 'TypeError' && error.message === 'terminated') {
return false
}

// We should log this exception
return true
}
21 changes: 1 addition & 20 deletions src/observability/middleware/handle-errors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { NextFunction, Response } from 'express'

import FailBot from '../lib/failbot'
import { shouldLogException, type ErrorWithCode } from '../lib/should-log-exception'
import { nextApp } from '@/frame/middleware/next'
import { setFastlySurrogateKey, SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key'
import { errorCacheControl } from '@/frame/middleware/cache-control'
Expand All @@ -9,26 +10,6 @@ import { ExtendedRequest } from '@/types'

const DEBUG_MIDDLEWARE_TESTS = Boolean(JSON.parse(process.env.DEBUG_MIDDLEWARE_TESTS || 'false'))

type ErrorWithCode = Error & {
code: string
statusCode?: number
status?: string
}

function shouldLogException(error: ErrorWithCode) {
const IGNORED_ERRORS = [
// Client connected aborted
'ECONNRESET',
]

if (IGNORED_ERRORS.includes(error.code)) {
return false
}

// We should log this exception
return true
}

async function logException(error: ErrorWithCode, req: ExtendedRequest) {
if (process.env.NODE_ENV !== 'test' && shouldLogException(error)) {
await FailBot.report(error, {
Expand Down
61 changes: 61 additions & 0 deletions src/observability/tests/handle-errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { describe, expect, test } from 'vitest'

import { shouldLogException, type ErrorWithCode } from '../lib/should-log-exception'

// Helper function to create test errors with code property
function createError(message: string, name: string = 'Error', code: string = ''): ErrorWithCode {
const error = new Error(message) as ErrorWithCode
error.name = name
error.code = code
return error
}

describe('shouldLogException', () => {
describe('ECONNRESET errors', () => {
test('should not log ECONNRESET errors', () => {
const error = createError('Connection reset', 'Error', 'ECONNRESET')

expect(shouldLogException(error)).toBe(false)
})
})

describe('TypeError: terminated filtering', () => {
test('should not log TypeError with exact message "terminated"', () => {
const error = createError('terminated', 'TypeError')

expect(shouldLogException(error)).toBe(false)
})

test('should log TypeError with different message', () => {
const error = createError('Cannot read property', 'TypeError')

expect(shouldLogException(error)).toBe(true)
})

test('should log TypeError with partial "terminated" message', () => {
const error = createError('connection terminated unexpectedly', 'TypeError')

expect(shouldLogException(error)).toBe(true)
})

test('should log non-TypeError with "terminated" message', () => {
const error = createError('terminated', 'Error')

expect(shouldLogException(error)).toBe(true)
})
})

describe('regular errors', () => {
test('should log regular errors', () => {
const error = createError('Something went wrong', 'Error')

expect(shouldLogException(error)).toBe(true)
})

test('should log errors with no code', () => {
const error = createError('Test error', 'Error')

expect(shouldLogException(error)).toBe(true)
})
})
})
Loading