Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[firebase_crashlytics] Handle case where function isn't in class for stack #1831

Merged
collinjackson merged 6 commits into from
Jul 11, 2019
Merged

[firebase_crashlytics] Handle case where function isn't in class for stack #1831

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d250f2
Handle case where no class in dart and android (ios is fine)
rmtmckenzie Jul 10, 2019
7fc4947
Bump version and update changlog
rmtmckenzie Jul 10, 2019
c5cf63e
Fix formatting of long line in FirebaseCrashlyticsPlugin.java
rmtmckenzie Jul 10, 2019
d15165f
Update firebase_crashlytics.dart
collinjackson Jul 10, 2019
142aaa3
Update FirebaseCrashlyticsPlugin.java
collinjackson Jul 10, 2019
b0fa819
Revert "Update FirebaseCrashlyticsPlugin.java"
collinjackson Jul 10, 2019
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
4 changes: 4 additions & 0 deletions packages/firebase_crashlytics/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.0.4+11

* Fixed an issue where `Crashlytics#getStackTraceElements` didn't handle functions without classes.

## 0.0.4+10

* Update README.
Expand Down
3 changes: 2 additions & 1 deletion ...o/flutter/plugins/firebase/crashlytics/firebasecrashlytics/FirebaseCrashlyticsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ private StackTraceElement generateStackTraceElement(Map<String, String> errorEle
String className = errorElement.get("class");
String methodName = errorElement.get("method");

return new StackTraceElement(className, methodName, fileName, Integer.parseInt(lineNumber));
return new StackTraceElement(
className == null ? "" : className, methodName, fileName, Integer.parseInt(lineNumber));
Copy link
Copy Markdown
Contributor

@collinjackson collinjackson Jul 10, 2019

Choose a reason for hiding this comment

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

I think className ?? "" would be more readable

Suggested change
className == null ? "" : className, methodName, fileName, Integer.parseInt(lineNumber));
className ?? "", methodName, fileName, Integer.parseInt(lineNumber));

Copy link
Copy Markdown
Contributor

@collinjackson collinjackson Jul 10, 2019

Choose a reason for hiding this comment

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

Oops, never mind, this is Java not Dart :)

Copy link
Copy Markdown
Contributor Author

@rmtmckenzie rmtmckenzie Jul 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Unless you know something I don't, I don't think that Java has a null coalescing operator yet... (oops, for some reason github didn't show me your second comment until after I wrote this)

Copy link
Copy Markdown
Contributor Author

@rmtmckenzie rmtmckenzie Jul 10, 2019

Choose a reason for hiding this comment

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

But I agree that's definitely more readable 😄

} catch (Exception e) {
Log.e(TAG, "Unable to generate stack trace element from Dart side error.");
return null;
Expand Down
26 changes: 17 additions & 9 deletions packages/firebase_crashlytics/lib/src/firebase_crashlytics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,25 @@ class Crashlytics {
final String lineNumber = lineParts[1].contains(":")
? lineParts[1].substring(0, lineParts[1].indexOf(":")).trim()
: lineParts[1];
final String className =
lineParts[2].substring(0, lineParts[2].indexOf(".")).trim();
final String methodName =
lineParts[2].substring(lineParts[2].indexOf(".") + 1).trim();

elements.add(<String, String>{
'class': className,
'method': methodName,

final Map<String, String> element = {
'file': fileName,
'line': lineNumber,
});
};

if (lineParts[2].contains(".")) {
final String className =
lineParts[2].substring(0, lineParts[2].indexOf(".")).trim();
final String methodName =
lineParts[2].substring(lineParts[2].indexOf(".") + 1).trim();

element['class'] = className;
element['method'] = methodName;
} else {
element['method'] = lineParts[2];
}

elements.add(element);
} catch (e) {
print(e.toString());
}
Expand Down
2 changes: 1 addition & 1 deletion packages/firebase_crashlytics/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: firebase_crashlytics
description: Flutter plugin for Firebase Crashlytics. It reports uncaught errors to the
Firebase console.
version: 0.0.4+10
version: 0.0.4+11
author: Flutter Team <flutter-dev@google.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/firebase_crashlytics

Expand Down
14 changes: 14 additions & 0 deletions packages/firebase_crashlytics/test/firebase_crashlytics_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,19 @@ void main() {
'line': '3825',
});
});

test('getStackTraceElements without class', () async {
final List<String> lines = <String>[
'package:firebase_crashlytics/test/main.dart 12 main'
];
final List<Map<String, String>> elements =
crashlytics.getStackTraceElements(lines);
expect(elements.length, 1);
expect(elements.first, <String, String>{
'method': 'main',
'file': 'package:firebase_crashlytics/test/main.dart',
'line': '12',
});
});
});
}