-
Notifications
You must be signed in to change notification settings - Fork 188
fix: centralize operation error handling #1071
base: master
Are you sure you want to change the base?
Changes from all commits
e06bbf3
d438e0e
f16f54d
846466e
4f24b7f
1396047
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||||||||
| import { capture } from '@snapshot-labs/snapshot-sentry'; | ||||||||||||
| import aliases from './aliases'; | ||||||||||||
| import follows from './follows'; | ||||||||||||
| import leaderboards from './leaderboards'; | ||||||||||||
|
|
@@ -23,8 +24,20 @@ import validations from './validations'; | |||||||||||
| import vote from './vote'; | ||||||||||||
| import votes from './votes'; | ||||||||||||
| import vp from './vp'; | ||||||||||||
| import log from '../../helpers/log'; | ||||||||||||
|
|
||||||||||||
| export default { | ||||||||||||
| const IGNORED_ERROR_CODES = ['ER_QUERY_TIMEOUT']; | ||||||||||||
|
|
||||||||||||
| function withErrorHandler(fn) { | ||||||||||||
| return (...args) => | ||||||||||||
| fn(...args).catch(e => { | ||||||||||||
| if (!IGNORED_ERROR_CODES.includes(e.code)) capture(e); | ||||||||||||
|
wa0x6e marked this conversation as resolved.
|
||||||||||||
| log.error(`[graphql] ${JSON.stringify(e)}`); | ||||||||||||
|
||||||||||||
| return Promise.reject(new Error('request failed')); | ||||||||||||
|
Member
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. Maybe need to figure out how we should handle custom error messages, for example vp.ts, user.ts |
||||||||||||
| }); | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+31
to
+38
|
||||||||||||
|
|
||||||||||||
| const operations = { | ||||||||||||
| space, | ||||||||||||
| spaces, | ||||||||||||
| ranking, | ||||||||||||
|
|
@@ -51,3 +64,7 @@ export default { | |||||||||||
| roles, | ||||||||||||
| leaderboards | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export default Object.fromEntries( | ||||||||||||
| Object.entries(operations).map(([key, fn]) => [key, withErrorHandler(fn)]) | ||||||||||||
|
||||||||||||
| Object.entries(operations).map(([key, fn]) => [key, withErrorHandler(fn)]) | |
| Object.entries(operations).map(([key, fn]) => { | |
| const skipWrapper = ['space', 'spaces', 'ranking', 'vote']; | |
| return skipWrapper.includes(key) ? [key, fn] : [key, withErrorHandler(fn)]; | |
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,9 @@ | ||
| import { capture } from '@snapshot-labs/snapshot-sentry'; | ||
| import log from '../../helpers/log'; | ||
| import db from '../../helpers/mysql'; | ||
|
|
||
| export default async function (parent, args) { | ||
| const id = args.id; | ||
| const query = `SELECT s.* FROM statements s WHERE id = ? LIMIT 1`; | ||
| try { | ||
| const statements = await db.queryAsync(query, id); | ||
| if (statements.length === 1) return statements[0]; | ||
| return null; | ||
| } catch (e: any) { | ||
| log.error(`[graphql] statement, ${JSON.stringify(e)}`); | ||
| capture(e, { args }); | ||
| return Promise.reject(new Error('request failed')); | ||
| } | ||
| const statements = await db.queryAsync(query, id); | ||
| if (statements.length === 1) return statements[0]; | ||
| return null; | ||
| } |
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.
The centralized error handler no longer captures context information (args, context, info) that was previously passed to capture() in the individual resolvers. This context is valuable for debugging and error tracking in Sentry. The wrapper has access to the arguments passed to each resolver function through the ...args parameter, but these aren't being passed to capture().
Consider capturing the arguments in the error handler to preserve this debugging context.
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.
this make sense as well. maybe we need them?