Skip to content

feat(errors): Added error handling hook for observability #101

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.

Sign up for GitHub

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

Closed
milkyontheblock wants to merge 2 commits into from
Closed

feat(errors): Added error handling hook for observability #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09397c8
feat(errors): add error handling hooks and improve error logging
May 11, 2026
fecc43c
chore(package): remove unused dependencies husky, lint-staged, and pr…
May 11, 2026
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
25 changes: 25 additions & 0 deletions demos/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict'

const gateway = require('./../index')
const PORT = process.env.PORT || 8080

const server = gateway({
routes: [{
prefix: '/*',
prefixRewrite: '',
target: 'http://127.0.0.1:3000',
hooks: {
onRequest: () => {
throw new SyntaxError('Something went wrong')
},
onError(err, req) {
// allows for extra error handling logic (i.e. Sentry, Newrelic etc.)
console.warn(req.method, req.url, err)
}
}
}]
})

server.start(PORT).then(server => {
console.log(`API Gateway listening on ${PORT} port!`)
})
Copy link
Copy Markdown
Collaborator

@molty3000 molty3000 May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor — Missing newline at end of file (\ No newline at end of file in the diff). POSIX convention.

1 change: 1 addition & 0 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ declare namespace fastgateway {
onRequest?: Function
rewriteHeaders?: Function
onResponse?: Function
onError?: (error: Error, request: Request) => void
Copy link
Copy Markdown
Collaborator

@molty3000 molty3000 May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: Wrong TypeScript type for req — Request is a browser/Fetch API type. In Node.js, the correct type is http.IncomingMessage. This will cause TypeScript compilation errors for users.

Either use http.IncomingMessage or stay consistent with the other hooks and use Function (matching the pattern of onRequest?: Function, onResponse?: Function above).

rewriteRequestHeaders?: Function
request?: {
timeout?: number
Expand Down
5 changes: 5 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@ const handler = (route, proxy, proxyHandler) => async (req, res, next) => {
proxyHandler(req, res, req.url, proxy, proxyOpts)
}
} catch (err) {
const { onError } = hooks
Copy link
Copy Markdown
Collaborator

@molty3000 molty3000 May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: Return value ignored — The hook is fire-and-forget. Users who want to handle the error themselves (return a custom response, log and suppress, etc.) have no way to signal that.

Consider: if onError returns true, skip the next(err) call so the route can produce a custom error response.

if (typeof onError === 'function') {
Copy link
Copy Markdown
Collaborator

@molty3000 molty3000 May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: No res parameter — The hook receives (err, req) but not res. Most observability integrations (Sentry, DataDog, NewRelic) benefit from response context — setting custom headers, logging status codes, or providing fallback error responses. Without res, users who want to return a custom error JSON body instead of the generic "Internal Server Error" can't do it here.

Consider adding res as a third parameter, or making this an async-compatible hook that can override the error response.

onError(err, req)
}

return next(err)
}
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"fast-proxy-lite": "^1.1.2",
"http-cache-middleware": "^1.4.1",
"micromatch": "^4.0.8",
"pump": "^3.0.4",
Copy link
Copy Markdown
Collaborator

@molty3000 molty3000 May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope creep — Adding pump as an explicit dependency and reordering devDependencies is unrelated to the error handling feature. These changes should be in a separate PR, or at minimum called out in the PR description. pump was already available as a transitive dependency — this is a cleanup, not part of the feature.

"restana": "^5.0.0",
"stream-to-array": "^2.3.0"
},
Expand All @@ -42,8 +43,8 @@
"LICENSE"
],
"devDependencies": {
"@types/node": "^22.13.11",
"@types/express": "^5.0.0",
"@types/node": "^22.13.11",
"artillery": "^2.0.21",
"aws-sdk": "^2.1691.0",
"chai": "^4.5.0",
Expand Down
Loading