fix(errors): Better define schema, align with python #1460
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hey @neilkakkar! 👋 |
|
Size Change: +3.15 kB (+0.11%) Total Size: 2.82 MB
ℹ️ View Unchanged
|
| export interface ErrorProperties { | ||
| $exception_type: string | ||
| $exception_message: string | ||
| $exception_level: SeverityLevel | ||
| $exception_list?: Exception[] | ||
| // $exception_source?: string | ||
| // $exception_lineno?: number | ||
| // $exception_colno?: number | ||
| $exception_DOMException_code?: string | ||
| $exception_is_synthetic?: boolean | ||
| // $exception_stack_trace_raw?: string | ||
| $exception_handled?: boolean | ||
| $exception_personURL?: string | ||
| $exception_language?: string | ||
| } |
There was a problem hiding this comment.
One for the future but would be cool to have a shared schema similar to what we have for TS <-> Python we have for queries
There was a problem hiding this comment.
will rm the commented out stuff 👀
| ...(lineno ? { $exception_lineno: lineno } : {}), | ||
| ...(colno ? { $exception_colno: colno } : {}), |
There was a problem hiding this comment.
Let's remove these from the other places they're referenced in a follow up PR
There was a problem hiding this comment.
they aren't referenced anywhere I think
There was a problem hiding this comment.
I think they're mentioned in the new Rust service. They're also in the taxonomy descriptions but that's less important because arguably they should stick around there...
There was a problem hiding this comment.
uh oh, that shouldn't be the case, it should use the values from the frame instead 🤔 - let me check again
There was a problem hiding this comment.
oh it's optional, all good, need it to stay there to support old versions that might send events 💀 , but yeah we don't need to use this at all imo
| @@ -177,13 +224,7 @@ export function errorToProperties([event, source, lineno, colno, error]: ErrorEv | |||
| $exception_level: isSeverityLevel(errorProperties.$exception_level) | |||
| ? errorProperties.$exception_level | |||
| : 'error', | |||
| ...(source | |||
| ? { | |||
| $exception_source: source, // TODO get this from URL if not present | |||
There was a problem hiding this comment.
I came here to ask if we should get rid of this... ahead of me :)
|
have changed things a decent amount, now the only top level prop is $exception_list and sometimes $exception_level.. |
| value: error.message, | ||
| mechanism: { | ||
| handled: true, | ||
| synthetic: false, |
There was a problem hiding this comment.
Should this be synthetic: true? We don't know for certain if it's just a string
There was a problem hiding this comment.
captureException only takes an error as a param though, not string 🤔 - and in this case we can't do stack resolution either, so not sure how we'd handle the synthetic case.. error.name would be undefined as well.
There was a problem hiding this comment.
Won't that only apply to people using Typescript? I guess someone could give us a string but I'm not even sure how we'd handle that right now... Let's revisit it once we have a few more examples. This can be the happy path for now 👍
| @@ -97,6 +97,7 @@ export function createEventProcessor( | |||
| // added manually to avoid any dependency on the lazily loaded content | |||
| $exception_message: any | |||
| $exception_type: any | |||
There was a problem hiding this comment.
i haven't yet removed $exception_message from the sentry integration, but eventually yes will do.
Changes
not json stringifying the stack, and aligning schema closer to sentry's / pythons, which should make inputting into cymbal easier as well (Olly lmk if this works well)
...
Checklist