ToBF 28.03 fixes #446
ToBF 28.03 fixes #446
Conversation
playground/frontend/playground_components/lib/src/cache/example_cache.dart
Show resolved
Hide resolved
|
|
||
| // ignore: avoid_catches_without_on_clauses | ||
| } catch (ex, trace) { | ||
| } catch (_) { |
There was a problem hiding this comment.
This way we lose details of the first exception when loading a precompiled object and only report the exception of loading a snippet. So far we do not use those details anyway and only report "Error loading example" but I do not like losing it.
Options are:
- Synthesize an exception to contain multiple exceptions, throw it here.
- Add both exceptions to
ToastNotifierhere. I do not like this because it mixes responsibilities.
Or we can just add a TODO to preserve both exceptions later.
There was a problem hiding this comment.
From completeError doc:
Calling [complete] or [completeError] must be done at most once.
Can exception and stackTrace be passed as arguments into _tryLoadSharedExample?
| _completer.complete(example); | ||
| } on Exception catch (ex, trace) { | ||
| _completer.completeError(ex, trace); | ||
| return; |
|
|
||
| class MultipleExceptions implements Exception { | ||
| final List<Exception> exceptions; | ||
|
|
There was a problem hiding this comment.
Add traces, they can also be useful.
| exampleCache.loadExampleInfo(example), | ||
| await exampleCache.loadExampleInfo(exampleBase), | ||
| ); | ||
| } on Exception catch (loadPrecompiledExampleException) { |
There was a problem hiding this comment.
| } on Exception catch (loadPrecompiledExampleException) { | |
| } on Exception catch (ex, trace) { |
No need for long names for local variables.
| _completer.completeError(ex, trace); | ||
| return; | ||
| Future<void> _tryLoadSharedExample({ | ||
| required Exception previousException, |
There was a problem hiding this comment.
A list was a better idea because that way the order of attempts can be changed easier.
Refinement of the tobf-all-tasks-28-03-23 branch.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.