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

Expose :value in SingleExceptionInterface #2071

Closed
jeffwidman opened this issue Jul 16, 2023 · 4 comments · Fixed by #2072
Closed

Expose :value in SingleExceptionInterface #2071

jeffwidman opened this issue Jul 16, 2023 · 4 comments · Fixed by #2072

Comments

@jeffwidman
Copy link
Contributor

jeffwidman commented Jul 16, 2023

👋 Hi from the :dependabot: team.

I'm attempting to migrate from sentry-raven to sentry-ruby:

With Raven we performed some basic client-side sanitization of the exception message:
https://github.com/dependabot/dependabot-core/blob/65f1c2d24e88ac57ed8df980871446deb5ea2d94/updater/lib/dependabot/sentry.rb#L5-L21

The unit tests illustrate how it works:
https://github.com/dependabot/dependabot-core/blob/65f1c2d24e88ac57ed8df980871446deb5ea2d94/updater/spec/dependabot/sentry_spec.rb#L24-L91

Migrating this from Raven to sentry-ruby's before_send is straightforward. However, it looks like sentry-ruby doesn't expose :value on SingleExceptionInterface:

attr_reader :type, :value, :module, :thread_id, :stacktrace

I see a recent change was made to expose :values on ExceptionInterface:

So I'm guessing there's no technical reason this can't be made publicly writable? Just no one has raised a feature request for it yet?

@jeffwidman
Copy link
Contributor Author

jeffwidman commented Jul 16, 2023

For now I've hacked around it using instance_variable_set but would much prefer having this publicly exposed:
https://github.com/dependabot/dependabot-core/pull/7553/files#diff-9ee3e6391d18c069de715a1740bb6239aa5f5c4734b3c03acd3455ba8bba3702R23-R24

@sl0thentr0py
Copy link
Member

yea I can expose it, PR obviously welcome to speed it up

@jeffwidman
Copy link
Contributor Author

Thanks, I put up a PR to expose it two weeks ago:

@sl0thentr0py
Copy link
Member

sorry didn't see that, need some time to catch up with this repo

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

Successfully merging a pull request may close this issue.

2 participants