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

feature : static SDK main entry withSentry.init(options),... #108

Merged
merged 18 commits into from
Oct 14, 2020

Conversation

rxlabz
Copy link
Contributor

@rxlabz rxlabz commented Oct 13, 2020

…captureEvent(...), .captureException(...)

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Add an Sentry class with static methods to init the sdk and capture the events.

💡 Motivation and Context

Sentry SDK unified API https://develop.sentry.dev/sdk/unified-api/#static-api

💚 How did you test it?

By checking if the internal client is correctly used when calling the static methods

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

more static methods

@rxlabz rxlabz requested a review from marandaneto October 13, 2020 11:23
Comment on lines 30 to 38
static Future<SentryResponse> captureEvent(
Event event, {
StackFrameFilter stackFrameFilter,
}) async {
return _client.captureEvent(
event: event,
stackFrameFilter: stackFrameFilter,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

our APIs return either a https://github.com/getsentry/sentry-java/blob/main/sentry/src/main/java/io/sentry/protocol/SentryId.java
or an empty SentryId which is 00000000-0000-0000-0000-000000000000 I guess.
Also we never take a stackFrameFilter as a param, we can work on this stuff later, but this will need to be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I rename the SentryResponse class

/// A response from Sentry.io.
///
/// If [isSuccessful] the [eventId] field will contain the ID assigned to the
/// captured event by the Sentry.io backend. Otherwise, the [error] field will
/// contain the description of the error.
@immutable
class SentryResponse {
const SentryResponse.success({@required this.eventId})
: isSuccessful = true,
error = null;
const SentryResponse.failure(this.error)
: isSuccessful = false,
eventId = null;
/// Whether event was submitted successfully.
final bool isSuccessful;
/// The ID Sentry.io assigned to the submitted event for future reference.
final String eventId;
/// Error message, if the response is not successful.
final String error;
to SentryId?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, not really, this is the whole response, we'd need to create a SentryId class and inside of the client, we just read the uuid out of the response and create a SentryId with the proper uuid, this doesn't need to be in this PR

@rxlabz rxlabz marked this pull request as ready for review October 13, 2020 15:49
@rxlabz rxlabz requested a review from bruno-garcia as a code owner October 13, 2020 15:49
@rxlabz rxlabz requested a review from marandaneto October 14, 2020 12:28
@rxlabz rxlabz force-pushed the feature/static-entry branch from 762545c to 02d0918 Compare October 14, 2020 13:59
@rxlabz rxlabz merged commit 5018978 into feature/unified-api Oct 14, 2020
@rxlabz rxlabz deleted the feature/static-entry branch October 14, 2020 15:41
@rxlabz rxlabz mentioned this pull request Oct 14, 2020
@marandaneto marandaneto mentioned this pull request Oct 15, 2020
8 tasks
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.

3 participants