dart-lang / async

A Dart package that contains tools to work with asynchronous computations.
https://pub.dev/packages/async
BSD 3-Clause "New" or "Revised" License
318 stars 50 forks source link

Make `ParallelWaitError` Include Error Details #256

Open caseycrogers opened 9 months ago

caseycrogers commented 9 months ago

The console and crash reporters (eg Firebase Crashlytics) use toString to display the error to the developer. But ParrallelWaitError just has a single sentence hardcoded string for toString: https://github.com/dart-lang/sdk/blob/0bc9815ad0e2aa508cd276cb2f1670b013f01b6e/sdk/lib/async/future_extensions.dart#L470

This means that almost no useful information is logged to console or (even worse) to crash reporting:

======== Exception caught by Flutter framework =====================================================
The following ParallelWaitError<(CustomerInfo?, bool?, Offering?), (AsyncError?, AsyncError?, AsyncError?)> was thrown:
ParallelWaitError

When the exception was thrown, this was the stack: 
====================================================================================================

image

The latter is especially bad because it makes it very hard to debug crashing parallel awaits in prod. Parallel waits contain multiple independent async function calls, so they're also just much more likely to be brittle than any single async function making it all the more important to have informative error messages

At the very least, ParallelWaitError should include sub-error details in toString. Ideally, the stack trace caught by Flutter would also include a stack trace for the first error in the list-but idk how stack traces are handled here so I don't really know what that'd look like. Here's one version of how toString could be improved, but there are a lot of options here and tbh it's a space I don't understand well so I'm more here to point out the user pain than to argue for an ideal solution. I'd be happy with anything better than the current behavior.

  String toString() {
   return 'ParallelWaitError: parallel wait finished with ${errors.nonNulls.length} errors.\n
     'Specific errors:\n'
     '${errors.nonNulls.map((e) => e.toString()).join('\n\n')}';
  };

Also note that the above requires making errors an iterable or handcoding some transform from the underlying record (in the case where it's a record) into a message.

caseycrogers commented 9 months ago

This is the best workaround I've come up with so far. It's still VERY non ideal. Biggest problem is the stack trace is still getting lost pretty badly.

class BetterParallelWaitError extends ParallelWaitError {
  BetterParallelWaitError._(super.values, super.errors);

  @override
  String toString() {
    return 'ParallelWaitError: parallel wait finished with '
        '1 or more errors.\n'
        'Errors:\n'
        '$errors';
  }
}

extension ParallelWaitErrorExtension on Object {
  // Inject this function into your crash reporter or into `onError`.
  Object get transformParallelError {
    if (this is ParallelWaitError) {
      final ParallelWaitError error = this as ParallelWaitError;
      return BetterParallelWaitError._(error.values, error.errors);
    }
    return this;
  }
}
final (int someInt, double someDouble, String someString) =
  await (
    () async { return 0; }(),
    () async { return null!; }(),
    () async { return 'foo'; }(),
).wait.onError((error, stackTrace) => throw error!.transformParallelError);
======== Exception caught by Flutter framework =====================================================
The following BetterParallelWaitError was thrown:
ParallelWaitError: parallel wait finished with 1 or more errors.
Errors:
(null, Null check operator used on a null value, null)

When the exception was thrown, this was the stack: 
#0      SubscriptionBloc.notifier.<anonymous closure>.<anonymous closure> (package:dribble_game_mobile/clients/dribble_purchases.dart:116:43)
<asynchronous suspension>
(elided 14 frames from dart:async)
====================================================================================================