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.

Sign up for GitHub

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

Jump to bottom

Fix newly surface errors after cleanup #64

Merged
merged 2 commits into from
Apr 21, 2020
Merged

Fix newly surface errors after cleanup #64

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
930c9d0
Fix newly surface errors after cleanup
natebosch Apr 13, 2020
199e5bb
Use Object in the typedef
natebosch Apr 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/src/chain.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class Chain implements StackTrace {
///
/// If [callback] returns a value, it will be returned by [capture] as well.
static T capture<T>(T Function() callback,
{void Function(Object error, Chain)? onError,
{void Function(dynamic error, Chain)? onError,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify why this should be dynamic now?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was dynamic before. I changed it in https://github.com/dart-lang/stack_trace/pull/62/files#diff-9e567343197420b24b39f009a31aa65aR76

My hope was that it wasn't breaking since any Function(dynamic, Chain) can work as a Function(Object, Chain), but I didn't change it in the typedef here:

stack_trace/lib/src/stack_zone_specification.dart

Line 14 in 90ea41b

typedef _ChainHandler = void Function(dynamic error, Chain chain);

I can either change that one too, or I need to change this one back to dynamic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that errors can't be null any more afaik, I think changing the typedef seems reasonable? Or is there some other reason we don't want to change that?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok. I'll go with that

bool when = true,
bool errorZone = true}) {
if (!errorZone && onError != null) {
Expand All @@ -82,7 +82,7 @@ class Chain implements StackTrace {
}

if (!when) {
void Function(Object, StackTrace) newOnError;
late void Function(Object, StackTrace) newOnError;
if (onError != null) {
void wrappedOnError(Object error, StackTrace? stackTrace) {
onError(
Expand All @@ -102,7 +102,7 @@ class Chain implements StackTrace {
return runZoned(() {
try {
return callback();
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
natebosch marked this conversation as resolved.
Show resolved Hide resolved
// TODO(nweiz): Don't special-case this when issue 19566 is fixed.
Zone.current.handleUncaughtError(error, stackTrace);

Expand Down
14 changes: 7 additions & 7 deletions lib/src/frame.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class Frame {
Frame parseLocation(String location, String member) {
var evalMatch = _v8EvalLocation.firstMatch(location);
while (evalMatch != null) {
location = evalMatch[1];
location = evalMatch[1]!;
evalMatch = _v8EvalLocation.firstMatch(location);
}

Expand All @@ -189,15 +189,15 @@ class Frame {
// lists anonymous functions within eval as "<anonymous>", while IE10
// lists them as "Anonymous function".
return parseLocation(
match[2],
match[2]!,
match[1]!
.replaceAll('<anonymous>', '<fn>')
.replaceAll('Anonymous function', '<fn>')
.replaceAll('(anonymous function)', '<fn>'));
} else {
// The second form looks like " at LOCATION", and is used for
// anonymous functions.
return parseLocation(match[3], '<fn>');
return parseLocation(match[3]!, '<fn>');
}
});

Expand All @@ -219,9 +219,9 @@ class Frame {
_catchFormatException(frame, () {
final match = _firefoxEvalLocation.firstMatch(frame);
if (match == null) return UnparsedFrame(frame);
var member = match[1].replaceAll('/<', '');
final uri = _uriOrPathToUri(match[2]);
final line = int.parse(match[3]);
var member = match[1]!.replaceAll('/<', '');
final uri = _uriOrPathToUri(match[2]!);
final line = int.parse(match[3]!);
if (member.isEmpty || member == 'anonymous') {
member = '<fn>';
}
Expand All @@ -233,7 +233,7 @@ class Frame {
var match = _firefoxSafariFrame.firstMatch(frame);
if (match == null) return UnparsedFrame(frame);

if (match[3].contains(' line ')) {
if (match[3]!.contains(' line ')) {
return Frame._parseFirefoxEval(frame);
}

Expand Down
6 changes: 3 additions & 3 deletions lib/src/stack_zone_specification.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ class StackZoneSpecification {

/// Looks up the chain associated with [stackTrace] and passes it either to
/// [_onError] or [parent]'s error handler.
void _handleUncaughtError(
Zone self, ZoneDelegate parent, Zone zone, error, StackTrace stackTrace) {
void _handleUncaughtError(Zone self, ZoneDelegate parent, Zone zone,
Object error, StackTrace stackTrace) {
if (_disabled) {
parent.handleUncaughtError(zone, error, stackTrace);
return;
Expand All @@ -163,7 +163,7 @@ class StackZoneSpecification {
// TODO(rnystrom): Is the null-assertion correct here? It is nullable in
// Zone. Should we check for that here?
self.parent!.runBinary(_onError!, error, stackChain);
} catch (newError, newStackTrace) {
} on Object catch (newError, newStackTrace) {
if (identical(newError, error)) {
parent.handleUncaughtError(zone, error, stackChain);
} else {
Expand Down
8 changes: 4 additions & 4 deletions test/chain/dart2js_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void main() {
expect(chain.traces, hasLength(2));
completer.complete();
}
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand Down Expand Up @@ -146,15 +146,15 @@ void main() {
}, onError: (error, chain) {
expect(error, equals('error'));
expect(chain.traces, hasLength(2));
throw error;
throw error as Object;
});
}, onError: (error, chain) {
try {
expect(error, equals('error'));
expect(chain, isA<Chain>());
expect(chain.traces, hasLength(2));
completer.complete();
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand All @@ -174,7 +174,7 @@ void main() {
expect(chain, isA<Chain>());
expect(chain.traces, hasLength(2));
completer.complete();
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand Down
10 changes: 5 additions & 5 deletions test/chain/vm_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void main() {
contains(frameMember(startsWith('inPeriodicTimer'))));
completer.complete();
}
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand Down Expand Up @@ -241,7 +241,7 @@ void main() {
expect(error, equals('error'));
expect(chain.traces[1].frames,
contains(frameMember(startsWith('inMicrotask'))));
throw error;
throw error as Object;
});
}, onError: (error, chain) {
try {
Expand All @@ -250,7 +250,7 @@ void main() {
expect(chain.traces[1].frames,
contains(frameMember(startsWith('inMicrotask'))));
completer.complete();
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand All @@ -271,7 +271,7 @@ void main() {
expect(chain.traces[1].frames,
contains(frameMember(startsWith('inMicrotask'))));
completer.complete();
} catch (error, stackTrace) {
} on Object catch (error, stackTrace) {
completer.completeError(error, stackTrace);
}
});
Expand Down Expand Up @@ -481,7 +481,7 @@ void main() {
'chain', () {
// Disable the test package's chain-tracking.
return Chain.disable(() async {
StackTrace trace;
late StackTrace trace;
await Chain.capture(() async {
try {
throw 'error';
Expand Down