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

Integrate advanced Bugsee features #49

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

koukibadr
Copy link
Contributor

GitHub Issue: #

Proposed Changes

  • Bug fix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes, no api changes)
  • Build or CI related changes
  • Documentation content changes
  • Other, please describe:

Description

  • Implement top-level Bugsee exception handler for dart and flutter (layout) exceptions
  • Implement data obscuration in reported videos
  • Add data obscuration flag saved in shared prefs (updated from the diagnostic overlay)
  • Custom logs with filters and enable/disable flag in shared prefs.
  • Enable custom events logged with reported exception
  • Attach custom traces with reported exceptions
  • Attach log file with the reported exceptions
  • Update diagnostic layout to test all Bugsee features (data obscuration, filter, events, traces, attributes...)

Impact on version

  • Major
    • The template structure was changed.
  • Minor
    • New functionalities were added.
  • Patch
    • A bug in behavior was fixed.
    • Documentation was changed.

PR Checklist

Always applicable

No matter your changes, these checks always apply.

  • Your conventional commits are aligned with the Impact on version section.
  • Updated CHANGELOG.md.
    • Use the latest Major.Minor.X header if you do a Patch change.
    • Create a new Major.Minor.X header if you do a Minor or Major change.
    • If you create a new header, it aligns with the Impact on version section and matches what is generated in the build pipeline.
  • Documentation files were updated according with the changes.
    • Update README.md and src/cli/CLI.md if you made changes to templating.
    • Update AzurePipelines.md and src/app/README.md if you made changes to pipelines.
    • Update Diagnostics.md if you made changes to diagnostic tools.
    • Update Architecture.md and its diagrams if you made architecture decisions or if you introduced new recipes.
    • ...and so forth: Make sure you update the documentation files associated to the recipes you changed. Review the topics by looking at the content of the doc/ folder.
  • Images about the template are referenced from the wiki and added as images in this git.
  • TODO comments are hints for next steps for users of the template and not planned work.

Contextual

Based on your changes these checks may not apply.

  • Automated tests for the changes have been added/updated.
  • Tested on all relevant platforms

Other information

Internal Issue (If applicable):

@koukibadr koukibadr requested review from jeanplevesque and Soap-141 and removed request for jeanplevesque November 21, 2024 15:16
@koukibadr koukibadr closed this Nov 22, 2024
@koukibadr koukibadr reopened this Nov 22, 2024
@koukibadr koukibadr marked this pull request as draft November 22, 2024 13:34
src/app/.env.dev Outdated Show resolved Hide resolved
Future<void> bugseeSetupTest() async {
testWidgets(
'Test Bugsee configuration',
(tester) async {},
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this is incomplete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes for this I've only prepared an integration test file for Bugsee we can later implement it
or I remove it for now ?

Copy link
Member

Choose a reason for hiding this comment

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

If it's not done, I would remove it. That way we don't have an incomplete test that could lead us to believe that Bugsee is tested even though it isn't.

src/app/.env.dev Outdated Show resolved Hide resolved
src/app/lib/access/bugsee/bugsee_configuration_data.dart Outdated Show resolved Hide resolved
src/app/lib/access/bugsee/bugsee_configuration_data.dart Outdated Show resolved Hide resolved
src/app/lib/access/bugsee/bugsee_configuration_data.dart Outdated Show resolved Hide resolved
src/app/lib/access/bugsee/bugsee_repository.dart Outdated Show resolved Hide resolved
src/app/lib/access/bugsee/bugsee_repository.dart Outdated Show resolved Hide resolved
src/app/lib/business/bugsee/bugsee_manager.dart Outdated Show resolved Hide resolved
@jeanplevesque
Copy link
Member

Make sure you also have a Bugsee.md file under the doc folder to explain how Bugsee is integrated.

@koukibadr koukibadr marked this pull request as ready for review November 28, 2024 11:36
doc/Bugsee.md Outdated Show resolved Hide resolved
doc/Bugsee.md Outdated Show resolved Hide resolved
doc/Bugsee.md Outdated Show resolved Hide resolved
@koukibadr koukibadr force-pushed the config/bk/integrate-advanced-bugsee branch from 139444c to 934f7f6 Compare December 2, 2024 11:00
@jeanplevesque jeanplevesque requested a review from a team December 2, 2024 20:42

This document provides a comprehensive guide to integrating and using **Bugsee** in your mobile application. Bugsee is a powerful tool for monitoring and debugging your app by capturing and reporting unhandled exceptions, providing insights into app crashes, user interactions, and more.

## **Overview**
Copy link
Member

Choose a reason for hiding this comment

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

Headers should not be bolded. Simply use the default header format.

@@ -4,5 +4,4 @@ MINIMUM_LEVEL='debug'
DAD_JOKES_BASE_URL='https://www.reddit.com/r/dadjokes'
APP_STORE_URL_IOS=https://apps.apple.com/us/app/uno-calculator/id1464736591
APP_STORE_URL_Android=https://play.google.com/store/apps/details?id=uno.platform.calculator
REMOTE_CONFIG_FETCH_INTERVAL_MINUTES=1
DIAGNOSTIC_ENABLED=true
Copy link
Member

Choose a reason for hiding this comment

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

Should DIAGNOSTIC_ENABLED=true be removed? I also have the same comment for the prod file too.

Future<void> bugseeSetupTest() async {
testWidgets(
'Test Bugsee configuration',
(tester) async {},
Copy link
Member

Choose a reason for hiding this comment

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

If it's not done, I would remove it. That way we don't have an incomplete test that could lead us to believe that Bugsee is tested even though it isn't.

}

final class BugseeConfigState extends Equatable {
/// Indicate if the app require a restart to reactivate the bugsee configurations
Copy link
Member

Choose a reason for hiding this comment

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

  • Indicates
  • requires
  • Add a period at the end.

/// cannot be true if `isBugseeEnabled == false`.
final bool isVideoCaptureEnabled;

/// Indicate if bugsee configuration is valid
Copy link
Member

Choose a reason for hiding this comment

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

Make sure you separate the sentences with a period and start sentences with a capital letter.

await registerBugseeManager();
runApp(const App());
},
GetIt.I.get<BugseeManager>().inteceptExceptions,
Copy link
Member

Choose a reason for hiding this comment

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

For the global exception handling, I think we should send them into a more generic class that would then send them into Bugsee, but we could also dispatch to other locations, such as the application logs and potentially other monitoring SDK (such as Crashlytics). For the scope of this PR, let's just start with Bugsee and the existing logging system (that logs into the console and a log file depending on the configuration).

@@ -5,6 +5,18 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)

Prefix your items with `(Template)` if the change is about the template and not the resulting application.

## 0.22.0
- Add bugsee global inteceptor for dart exceptions to template app
Copy link
Member

Choose a reason for hiding this comment

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

Again, sentences should end with a period. (I know some previous notes are missing, but if we look at the initial ones, they were correct.)

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.

2 participants