Do Notation throws instance of '_OptionThrow'

[zellidev0]

In version 1.1.0 of fpdart:

The following code snippet is a simplified version of a piece of code that had a runtime error.
I assume, because I'm mapping over the List of testOption the Option.Do constructor does not catch the throwing of _OptionThrow.

I assumed that the exception would be caught by the do Constructor, but this was not the case.

Here is the problematic snippet:

  test('test', () async {
    Option<Iterable<String?>> testOption = const Option<List<String?>>.of(
      ['/', '/test', null],
    );
    Option<Iterable<Uri>> doNotation = Option.Do(
      ($) => $(testOption).map(
        (String? stringValue) => $(
          optionOf(
            Uri.tryParse($(optionOf(stringValue))), //
          ),
        ),
      ),
    );
    expect(doNotation, equals(const Option<Uri>.none()));

Here is the thrown exception:

Instance of '_OptionThrow'
package:fpdart/src/option.dart 45:28                                   _doAdapter.<fn>
package:fpdart/src/option.dart 589:72                                  None.match
package:fpdart/src/extension/option_extension.dart 29:39               FpdartOnOption.getOrElse
package:fpdart/src/option.dart 45:12                                   _doAdapter
test/implementation_test.dart 197:27  main.<fn>.<fn>.<fn>
dart:core                                                              _StringBase._interpolate
package:fpdart/src/option.dart 558:39                                  Some.toString
package:matcher                                                        expect
package:flutter_test/src/widget_tester.dart 454:18                     expect
test/implementation_test.dart 202:5   main.<fn>

Here is a rewrite without a callback that correctly catches the thrown exception and returns none

  test('test2', () async {
    Option<Iterable<String?>> testOption = const Option<List<String?>>.of(
      ['/', '/test', null],
    );
    Option<Iterable<Uri>> doNotation = Option.Do(($) {
      List<Uri> list = [];
      for (final String? stringValue in $(testOption)) {
        final uri = $(optionOf(Uri.tryParse($(optionOf(stringValue)))));
        list.add(uri);
      }
      return list;
    });

    expect(doNotation, equals(const Option.none()));
  });

This is pretty dangerous, because I don't have the type safety at compile time but have a runtime error I totally oversaw in the code review.

How should I proceed with this?
Could the Do constructors implementation be fixed so that callbacks can be used inside, or is this a language limitation and I therefore can't use the Do constructor when Using callbacks (which would be hard, because mapping lists etc is not possible anymore)?

Thanks.

[SandroMaglione]

Hi @zellidev0

The $ function inside the Do notation cannot be used nested inside a function callback (.map in your example) or in a for condition.

Instead, I would suggest to refactor your code as follows (I added a test for Option):

fpdart/packages/fpdart/test/src/option_test.dart

Lines 710 to 729 in d2d21ad

	test('should traverse over a list', () async {
	const testOption = const Option<List<String?>>.of(
	['/', '/test', null],
	);
	Option<List<Uri>> doNotation = Option.Do(
	($) {
	List<String?> optionList = $(testOption);
	return $(optionList.traverseOption(
	(stringValue) => optionOf(stringValue).flatMap(
	(uri) => optionOf(
	Uri.tryParse(uri),
	),
	),
	));
	},
	);
	expect(doNotation, equals(const Option<Uri>.none()));
	});

$ allows to extract the value of an Option inside the .Do constructor function.

[zellidev0]

@SandroMaglione thanks for the response.

This is unfortunate because I have to be careful not to accidentally use a callback in a do constructor and the compiler isn't telling me I'm creating a exception.

When not using the do constructor the compiler would tell me if I'm extracting the content of a option etc.
Basically I'm transforming a compile time error to a runtime error. Do you agree?

Do you have any plans in creating a version of the do constructor that creates a compile time error in these cases? And if not should we update the docs of the do constructor part in the package documentation? Because If i hadn't implemented it that way I would not have found out that this is an issue.

[SandroMaglione]

@zellidev0 the Do notation helps to make the code more readable but comes with some trade offs (reference to the full discussion).

You are technically allowed to execute code that can error, but once you know what should be avoided the Do notation becomes really convenient.

Some patterns to avoid are:

• Throwing inside Do constructor
• Using await without executing the $ function
• Not using $ nested

I agree that we should add these points to the documentation. Mind opening a PR for this?

[zellidev0]

@SandroMaglione that's a good idea.
Please assign the issue to me.

I created a Pull request here.

[danielRi]

Hey!

I got the exact same issue, but apart from changing an Either to an option and using async/await I wouldnt know why this isnt a normal example where the code just jumps to the getOrElse() part, instead throws _OptionThrow.

Future<ResultState> _handleInit(
      Init event, Emitter<ResultState> emit) {
    return Option.Do((_) async {
      final vin = _(_selectedVehicleRepo.vin);
      final formData = _(_formDataRepo.load());
      final either = await _actionRepo.parseArguments(formData);
      print(either); // prints Left(error), as expected
      final optionActionResponse = Option.fromEither(either);
      print(optionActionResponse); // prints None, as expected
      final actionResponse = _(optionActionResponse); // throws Error: Instance of '_OptionThrow'
      return ResultState.loaded(
        result: '123',
      );
    }).getOrElse(() async {
      return ResultState.error(LocaleKeys.exception_thrown.tr());
    });
  }

I think I dont violate any of these rules here, or am I mistaken (not sure about the 2nd one):

Throwing inside Do constructor
Using await without executing the $ function
Not using $ nested

Exptected behavior:

I would expect here the code just "jumps" to the getOrElse part, because an Option returns None.

[zellidev0]

@danielRi I think the issue is with using a async callback in the Option.Do(...) constructor.
When having to use async you should use a Task and the TaskEither etc. types.

Task<ResultState> _handleInit(
  Init event,
  Emitter<ResultState> emit,
) => Option.Do((_) async {
    final vin = _(_selectedVehicleRepo.vin);
    final formData = _(_formDataRepo.load());
  })
      .toEither(() => 'Error extracting vehicleRepoVin or formData')
      .toTaskEither()
      .map((formData) => _actionRepo.parseArguments(formData))
      .match(
        (_) => ResultState.error(LocaleKeys.exception_thrown.tr()),
        (result) => ResultState.loaded(
          result: result,
        ),
      );

Also: You don't need the ResultState, you can simply return a TaskEither<Object,YourDataType> from the function.
You can omit the match in this case and do the LocaleKeys... in your view logic when calling match.

[SandroMaglione]

@danielRi as @zellidev0 mentioned the issue is using async inside Do without _:

final either = await _actionRepo.parseArguments(formData); // Use `_` here

Another suggestion is to remember not to use print inside pure code. print causes side effects.