-
-
Notifications
You must be signed in to change notification settings - Fork 658
Add Sentry for error reporting #443
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
Rubocop violations
Generated by 🚫 Danger |
Codecov Report
@@ Coverage Diff @@
## master #443 +/- ##
==========================================
- Coverage 92.23% 92.06% -0.18%
==========================================
Files 319 319
Lines 16393 16492 +99
==========================================
+ Hits 15120 15183 +63
- Misses 1273 1309 +36
Continue to review full report at Codecov.
|
Rakefile
Outdated
| @@ -100,6 +135,16 @@ def storage | |||
| ) | |||
| end | |||
|
|
|||
| def with_sentry_dsn | |||
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.
The sentry_dsn is a secret so this piece of code makes sure the value is defined in the SentryDsn.swift file when packaging a new release of Tuist.
| /// Returns true if the user decided to disable the analytics by setting the TUIST_ANALYTICS_DISABLED environment variable. | ||
| /// | ||
| /// - Returns: True if the analytics are disabled. | ||
| func isDisabled() -> Bool { |
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.
cc @kwridan do you think this should be enough?
…the executable directory is part of the dynamic linker paths in which it looks for the frameworks to link
|
The PR is blocked by this issue. We need the Sentry SDK API to provide an interface to send all the events and subscribe to the completion of the delivery. That'll allow Tuist to wait until the events are reported, otherwise it exits before the events reach the server. |
|
After some tests, I realized that the Swift Package Manager doesn't export the dSYMs and therefore the stacktraces in Sentry can't be desymbolicated. That complicates debugging the issue. I suggest taking a different approach here. Instead of reporting the errors to Sentry, use the |
|
Agreed, I think a simpler alternate is better suited here - from @ollieatkinson suggestions (from slack) we can start by catching / surfacing the errors in the command line that will aid better bug reports. |
|
The suggestion on slack was to create a global function for tagging errors which can be propagated through standard swift error handling. @discardableResult @inlinable
public func report<T>(_ block: @autoclosure () throws -> T, message: String = "", file: String = #file, line: Int = #line, function: String = #function) throws -> T {
do {
return try block()
} catch let error as ReportedError {
throw error
} catch let error as FatalError {
throw ReportedError(error: error, message: message, file: file, line: line, function: function)
} catch let error {
throw ReportedError(error: UnhandledError(error: error), message: message, file: file, line: line, function: function)
}
}We should expand on this further and provide support for capturing the output of the shell command which failed. |
|
I'll open a PR following the suggested approach. |
Short description 📝
Handled bugs and unhandled errors are not being reported at the moment. Therefore, we can only know about them if developers open issues on this repository.
Solution 📦
Create a project on Sentry and set up Tuist to use it.
Implementation 👩💻👨💻
TuistCoretarget against it.ErrorHandlerlogic to report bugs.packagetask to override the linking settings of thetuistandtuistenvbinaries to link dynamically againstSentry.framework.Sentry.frameworkSentry.frameworknext totuistenv.Setup code