Skip to content

Mark transaction as internal_error in case of unhandled errors#1218

Merged
marandaneto merged 5 commits into
v7.0.0from
chore/internalerror-transaction
Jan 12, 2023
Merged

Mark transaction as internal_error in case of unhandled errors#1218
marandaneto merged 5 commits into
v7.0.0from
chore/internalerror-transaction

Conversation

@marandaneto

@marandaneto marandaneto commented Jan 10, 2023

Copy link
Copy Markdown
Contributor

📜 Description

💡 Motivation and Context

Closes #1182

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@codecov-commenter

codecov-commenter commented Jan 11, 2023

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (v7.0.0@dfa335f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
dart/lib/src/isolate_error_integration.dart 88.88% 2 Missing ⚠️
dart/lib/src/run_zoned_guarded_integration.dart 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             v7.0.0    #1218   +/-   ##
=========================================
  Coverage          ?   89.76%           
=========================================
  Files             ?      111           
  Lines             ?     3489           
  Branches          ?        0           
=========================================
  Hits              ?     3132           
  Misses            ?      357           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread CHANGELOG.md

## 7.0.0-alpha.1

### Various fixes & improvements

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just fixing the changelog.

);
}
/// Parse and raise an event out of the Isolate error.
@visibleForTesting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just moved from a standalone function to inside of the class.

Comment on lines +70 to +74
// marks the span status if none to `internal_error` in case there's an
// unhandled error
hub.configureScope((scope) => {
scope.span?.status ??= const SpanStatus.internalError(),
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's the added code.

/// Needed to check if we somehow caused a `print()` recursion
bool _isPrinting = false;

@visibleForTesting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just moved to the class, to be tested.

Comment on lines +50 to +54
// marks the span status if none to `internal_error` in case there's an
// unhandled error
hub.configureScope((scope) => {
scope.span?.status ??= const SpanStatus.internalError(),
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added code.


void main() {
TestWidgetsFlutterBinding.ensureInitialized();
group(OnErrorIntegration, () {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just moved within the group

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.handled, true);
});
test('marks transaction as internal error if no status', () async {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added test

@marandaneto
marandaneto marked this pull request as ready for review January 11, 2023 14:10

@brustolin brustolin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@marandaneto
marandaneto merged commit 795f47f into v7.0.0 Jan 12, 2023
@marandaneto
marandaneto deleted the chore/internalerror-transaction branch January 12, 2023 09:42
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