sentry-java
sentry-java copied to clipboard
Align ignoredExceptionsForType naming on Java and .NET
On Java, its addIgnoredExceptionForType(x)
on .NET AddExceptionFilterForType(x)
https://docs.sentry.io/platforms/dotnet/usage/#ignoring-errors
I like the AddExceptionFilterForType
I agree with the motivation behind aligning names of options and other things across SDKs. We should have that as a goal for anything we do.
With regards of changing these methods, I find the drawback of introducing a breakage on our users for the sake of 'aligning the name with .NET" it's not really worth it. If we add the second name with a Obsolete
annotation on it indicating the users should use the new name, meaning no breaking code change, I'm more in favor of it but just "deleting" the method and writing the new one on the Migration Guide is too much friction and not worth the benefit.
Worth noting if the docs don't have the right name, we should amend that.
So Java is already documented correctly https://docs.sentry.io/platforms/java/configuration/options/#ignored-exceptions-for-type
Usually, we don't do breaking changes before annotating the method with @Deprecated
and @ScheduledForRemoval
so folks have time to migrate before getting a compile error.
I'm fine either way, I'm not a big fan of special case naming per platform because the docs become a little bit of a nightmare to be maintained because of all duplications, that's why we have a Unified API as well.
Also, Usually, it's a single person answering all the questions from multiple sources about "why this does not exist?" and you have to dig into the source code to know where and why it's under a different name for any reason, sometimes even because its a reserved word, a simple and fresh example https://github.com/getsentry/sentry-react-native/issues/2213
That said, both docs are up to date with the naming, we can decide to close this issue if you are all fine with it, but I vote for aligning them, this is not a priority though, that's why the milestone v6 has been removed on the 10th of March, so it's not a blocker for the Java release v6.
Suggested course of action: Deprecate existing option and add new option Then I guess for 7.0 we can remove the deprecated option.
Suggested course of action: Deprecate existing option and add new option Then I guess for 7.0 we can remove the deprecated option.
Yes, that's what we always did, example https://github.com/getsentry/sentry-java/pull/1873/files
After looking the the .NET code here https://github.com/getsentry/sentry-dotnet/blob/720e8d1aa75f1603f7712242bb3fcf365917ee19/src/Sentry/SentryOptionsExtensions.cs#L116 and here https://github.com/getsentry/sentry-dotnet/blob/720e8d1aa75f1603f7712242bb3fcf365917ee19/src/Sentry/Extensibility/IExceptionFilter.cs this is not just a naming difference but also differs in how this is implemented.
Java uses a simple list of classes to be ignored whereas .NET allows for more complex filters as well (https://github.com/getsentry/sentry-dotnet/blob/720e8d1aa75f1603f7712242bb3fcf365917ee19/src/Sentry/SentryOptionsExtensions.cs#L106).
Feels like the current naming in Java fits its implementation quite well, so how about just adding addExceptionFilterForType
without deprecating the existing addIgnoredExceptionForType
? Then we support the common name from other SDKs and can document it.
@adinauer for me, they look exactly the same (the issue is about the ignoredExceptionsForType method), See code snippet https://docs.sentry.io/platforms/dotnet/usage/#ignoring-errors https://docs.sentry.io/platforms/java/configuration/#ignored-exceptions-for-type
Yes the way to add it is the same because .NET has this method to make it easier to add a filter for a type without having to create the filter yourself. My point is that renaming everything in the Java SDK doesn't seem right because it's not really a list of filters but just a list of classes. To me that would make the naming misleading.
So my new suggestion is to just add the method to align SDKs regarding how to add an Exception that should be ignored but keep the old naming in place and also keep all the internal naming for this.