-
-
Notifications
You must be signed in to change notification settings - Fork 253
Feat/dart default integrations #187
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
Conversation
Co-authored-by: Manoel Aranda Neto <[email protected]>
dart/example_web/web/main.dart
Outdated
error, | ||
stackTrace: stackTrace, | ||
); | ||
final sentryId = await Sentry.captureException(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the stackTrace: stackTrace
removed because error
is an Error
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was, but now that I put it back, the captureException fails in release mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm, did you figure this out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so after investigations :
in browser release mode, capturing the error without adding the stacktrace works, but fails with the optional stacktrace if it is passed as stacktrace, but works if it is passes as String :)
- this works in web release mode
final sentryId = await Sentry.captureException(
error,
stackTrace: stackTrace.toString(),
);
- this fails in web release mode
final sentryId = await Sentry.captureException(
error,
stackTrace: stackTrace,
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have an idea why this happens?
we could add a condition like:
stackTrace: (stackTrace != null) ? ((isWeb) ? stackTrace.toString() : stackTrace) : null
adding a comment why of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn thats problematic, let's fix for Chrome and add a comment for Safari
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as it is it works on Chrome, where would be the better place for a comment ? in the readme ? or StacktraceFactory ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both I'd say, but please create a task for this too, (to research and fix it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
task was not created, so I did myself
https://github.com/getsentry/sentry-dart/projects/1#card-50215638
Co-authored-by: Manoel Aranda Neto <[email protected]>
dart/example/main.dart
Outdated
@@ -82,6 +88,8 @@ Future<void> main() async { | |||
} finally { | |||
await Sentry.close(); | |||
} | |||
|
|||
exit(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this if we await
the events to be sent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, without it the dart process seems to never end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dart does not end the process or it gets a deadlock somewhere in our SDK? worth checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems to be caused by the isolateErrorIntegration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ I removed the exit
call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but the problem still exists or what? just to know if you found out or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the problem still exists. it's caused by the isolateErrorIntegration, but didn't found how to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
task was not created, so I did myself
https://github.com/getsentry/sentry-dart/projects/1#card-50215663
- install flutterErrorIntegration before runZonedGuardedIntegration
Co-authored-by: Manoel Aranda Neto <[email protected]>
Co-authored-by: Manoel Aranda Neto <[email protected]>
…/sentry-dart into feat/dart-default-integrations
Codecov Report
@@ Coverage Diff @@
## main #187 +/- ##
==========================================
- Coverage 78.93% 78.36% -0.58%
==========================================
Files 45 47 +2
Lines 1486 1428 -58
==========================================
- Hits 1173 1119 -54
+ Misses 313 309 -4
Continue to review full report at Codecov.
|
📢 Type of change
📜 Description
isolateErrorIntegration
andrunZoneGuardedIntegration
to dart default integrationsinitialIntegrations
toSentry.init
to allow running custom integrations beforerunZoneGuardedIntegration
💡 Motivation and Context
💚 How did you test it?
new unit tests
📝 Checklist
🔮 Next steps