rollbar / rollbar-flutter

Rollbar for Dart and Flutter
https://docs.rollbar.com/docs/flutter
MIT License
21 stars 27 forks source link

Library lacks proper type-safety, immutability and by extension "null-safety" #43

Closed matux closed 2 years ago

matux commented 2 years ago

Our data structures assumes all values can be null and our code does little to protect itself from null values, data structures are also fully mutable and side-effects are indiscriminately mutating them without setting mutability boundaries.

This is very error prone and very hard, makes our code very hard to reason about and adds unnecessary complexity to maintainability.

A data structure instead of looking like this:

class Client {
  String? locale;
  String? hostname;
  String? os;
  String? osVersion;
  String? rootPackage;
  Map<String, String>? dart;

It should look like this:

@sealed
@immutable
class Client {
  final String locale;
  final String hostname;
  final String os;
  final String osVersion;
  final String rootPackage;
  final Map<String, String> dart;

By guaranteeing these contracts for our data structures, we remove entire classes of errors from our code, we make it easier to reason about.

Without type/null-safety, our code ends up looking like this:

void _restoreDartChainMessage(
    List<TraceInfo?>? dartChain, List<TraceInfo?> embeddedChain) {
  if (embeddedChain.isNotEmpty) {
    for (var element in dartChain!) {
      if (element!.exception != null &&
          element.exception!.message!.startsWith('PlatformException') &&
          element.exception!.message!.contains(androidTracePayloadPrefix)) {
        element.exception!.message =
            'PlatformException(error, "${embeddedChain[0]!.exception!.message}")';
      }
    }
  }
}

Were we have to check for nullness and when not, make dangerous assumptions that could crash our library: https://github.com/rollbar/rollbar-flutter/pull/33.

By embracing type-safety, immutability and boundaries around side-effects, we reduce the chance of code errors, and we won't have to pay for them with crashing our user's applications.