Skip to content
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.

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

rfc(feature): Exception Groups #79

Merged
merged 23 commits into from
Apr 7, 2023
Merged

Conversation

mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Mar 7, 2023

Design for changes required to support exception groups.

Rendered RFC

text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Show resolved Hide resolved
@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review March 26, 2023 23:17
text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Outdated Show resolved Hide resolved
Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
@mattjohnsonpint mattjohnsonpint marked this pull request as draft March 27, 2023 15:52
@mattjohnsonpint
Copy link
Contributor Author

mattjohnsonpint commented Apr 1, 2023

Yet another major rewrite of this RFC. I believe the plan is solid now 🤞 😅

Please read the revised proposal, starting from the "Proposed Solution" section. (Everything prior is as before.)

@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review April 1, 2023 03:54
text/0079-exception-groups.md Outdated Show resolved Hide resolved
text/0079-exception-groups.md Outdated Show resolved Hide resolved
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

This new approach looks good to me, and makes sense on the SDK side. Thanks for keeping this going @mattjohnsonpint!

@mattjohnsonpint mattjohnsonpint merged commit 2ade5dd into main Apr 7, 2023
@mattjohnsonpint mattjohnsonpint deleted the rfc/exception-groups branch April 7, 2023 16:22
@lforst
Copy link
Member

lforst commented Jul 4, 2023

An a posteriori learning: Extending an interface, which in itself is optional but has required fields (i.e. mechanism) is really painful. What if the exception I want to put an exception_id on doesn't have a mechanism yet? I would now have to put nonsensical default values inside mechanism to conform to the protocol again.

Maybe we should have put all the extra data onto the exception interface directly 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants