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

Fail gracefully when no API key is provided #114

Closed
shalzz opened this issue Jul 28, 2016 · 7 comments
Closed

Fail gracefully when no API key is provided #114

shalzz opened this issue Jul 28, 2016 · 7 comments

Comments

@shalzz
Copy link

shalzz commented Jul 28, 2016

Instead of throwing an NPE just log an error so that we can use the library in an open source project without an api key and not get tons of debug crashes from unknown sources.

@JakeWharton
Copy link
Contributor

Why not only initialize when it's a release build? Here's an example:
https://github.com/JakeWharton/Telecine/blob/master/telecine/src/main/java/com/jakewharton/telecine/TelecineApplication.java

I don't want to use a crash reporting library that silently fails to
initialize.

On Thu, Jul 28, 2016, 6:57 AM Shaleen Jain notifications@github.com wrote:

Instead of throwing an NPE just log an error so that we can use the
library in an open source project without an api key and not get tons of
debug crashes from unknown sources.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#114, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAEEEeWY-run1UobDmttasONp6Mj9NCaks5qaKdMgaJpZM4JXLvZ
.

@shalzz
Copy link
Author

shalzz commented Jul 31, 2016

Yes, but then you'd have to guard every other bugsnag static method as well. That's a lot of boilerplate.

It won't really be silent if it logs an error. Besides if you can't follow the setup instructions then you really shouldn't be using a crash reporting library. Their docs are pretty good. ;)

@JakeWharton
Copy link
Contributor

A log is effectively silent. And it's not incompetence I'm worried about,
it's accidental refactorings in code or the build config which will cause
the key to become absent and go unnoticed instead of instantly failing.

I don't find it unreasonable to guard your calls or use an abstraction. I
wouldn't expect an analytics library or payment SDK or social SDK to
silently do nothing either when a required piece of information was not
provided.

This library requires that piece of information for all functionality yet
it's your app that wants the ability to have a no-op implementation. It
doesn't seem right to force that onto the library then. If you want the
interface/implementation abstraction to be part of this library I think
that makes sense, but the no-op version does not seem appropriate.

On Sun, Jul 31, 2016, 3:14 AM Shaleen Jain notifications@github.com wrote:

Yes, but then you'd have to guard every other bugsnag static method as
well. That's a lot of boilerplate.

It won't really be silent if it logs an error. Besides if you can't follow
the setup instructions then you really shouldn't be using a crash reporting
library. Their docs are pretty good. ;)


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#114 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEEdnftO90jHCkPh_ZzUNPO_IoJxvVks5qbEtLgaJpZM4JXLvZ
.

@shalzz
Copy link
Author

shalzz commented Jul 31, 2016

You can always write unit tests to catch regressions in your code.

Libraries should be plug-able and not create hard dependencies, I should be able to turn its functionality on/off without having to go through my entire code base just for testing or performance analysis.

For eg, Google Analytics doesn't crash your whole application if something isn't properly configured, even if that includes not adding the api key, instead it is flexible and lets the developer decide how to use the library including the ability to enable/disable it.

At the very least all the other bugsnag methods shouldn't throw an exception if bugsnag is initialized only on release builds.

@JakeWharton
Copy link
Contributor

Great. It sounds like you agree with me that the library should provide an
abstraction for your application requirements but not implement your
requirements directly.

On Sun, Jul 31, 2016, 4:35 AM Shaleen Jain notifications@github.com wrote:

You can always write unit tests to ensure your changes in code does not
cause regressions.

Libraries should be plug-able and not create hard dependencies, I should
be able to turn its functionality on/off without having to go through my
entire code base just for testing or performance analysis.

For eg, Google Analytics doesn't crash your whole application if something
isn't right even if you don't include the api key, instead it is flexible
and lets the developer decide how to use the library including the ability
to enable/disable it.

At the very least all the other bugsnag methods shouldn't throw an
exception if bugsnag is initialized only on release builds.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#114 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEEeCqfIIMoi2PdwtbAsvo9avggX2sks5qbF5UgaJpZM4JXLvZ
.

@shalzz
Copy link
Author

shalzz commented Aug 2, 2016

Yeah.

On Mon 1 Aug, 2016, 9:30 AM Jake Wharton, notifications@github.com wrote:

Great. It sounds like you agree with me that the library should provide an
abstraction for your application requirements but not implement your
requirements directly.

On Sun, Jul 31, 2016, 4:35 AM Shaleen Jain notifications@github.com
wrote:

You can always write unit tests to ensure your changes in code does not
cause regressions.

Libraries should be plug-able and not create hard dependencies, I should
be able to turn its functionality on/off without having to go through my
entire code base just for testing or performance analysis.

For eg, Google Analytics doesn't crash your whole application if
something
isn't right even if you don't include the api key, instead it is flexible
and lets the developer decide how to use the library including the
ability
to enable/disable it.

At the very least all the other bugsnag methods shouldn't throw an
exception if bugsnag is initialized only on release builds.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
<
#114 (comment)
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AAEEEeCqfIIMoi2PdwtbAsvo9avggX2sks5qbF5UgaJpZM4JXLvZ

.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#114 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADuexYtGHOH2FHAhXenrQOcOWyglDNTJks5qbW91gaJpZM4JXLvZ
.

@kattrali
Copy link
Contributor

I looked at a few possibilities for this issue, and I think the best solutions are ones which can be done already:

  1. Always set the API key and configure notifyReleaseStages to exclude debug builds. This is probably the easiest solution, as internally the Bugsnag client ignores any notification received when the configured releaseStage is not present in notifyReleaseStages. Committing the API key to an open source project has no data exposure consequences, as it is a write-only key and cannot be used to access the project’s data.
  2. Conditionally configure Bugsnag within a wrapper. This is commonly done when Bugsnag is used along with other analytics/logging tools, and reduces overhead when tweaking the library usage as a whole.

lemnik pushed a commit that referenced this issue Jun 2, 2021
fix: Update ProGuard keep rules for Breadcrumb class
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

No branches or pull requests

3 participants