Bug: Custom exception not thrown

[marcglasberg]

Celest version: 0.2.0-dev.3

Hey guys, the tests of my Celest demonstration app caught an error. I just verified it's actually not in the app code, but in the Celest code.

If I write this function definition it works and throws MyException:

Future<({Stock stock, CashBalance cashBalance})> sellStock(AvailableStock availableStock, { required int howMany }) async {
  throw MyException('Cannot sell stock you do not own');
}

However, if I change it like the following, instead of throwing MyException, it throws a BadRequestException("MyException"):

Future<({Stock stock, CashBalance cashBalance})> sellStock(AvailableStock availableStock, { required int howMany }) async {
  _throwSomeException();
}

void _throwSomeException() {
  throw MyException('Cannot sell stock you do not own');
}

In the first case, the generated code is this:

  Future<({CashBalance cashBalance, Stock stock})> sellStock(AvailableStock availableStock, {required int howMany}) async {
    final $response = await celest.httpClient.post(
    ......
    switch ($code) {
      case r'MyException':
        throw Serializers.instance.deserialize<MyException>($details);
      case r'BadRequestException':
        throw Serializers.instance.deserialize<BadRequestException>($details);
      case r'InternalServerException':
        throw Serializers.instance
            .deserialize<InternalServerException>($details);
      case _:
        switch ($response.statusCode) {
          case 400: throw BadRequestException($code);
          case _: throw InternalServerException($code);
        }  }  }

But the second one is missing the MyException serialization:

  Future<({CashBalance cashBalance, Stock stock})> sellStock(AvailableStock availableStock, { required int howMany}) async {
    .......
    switch ($code) {
      case r'BadRequestException':
        throw Serializers.instance.deserialize<BadRequestException>($details);
      case r'InternalServerException':
        throw Serializers.instance
            .deserialize<InternalServerException>($details);
      case _:
        switch ($response.statusCode) {
          case 400: throw BadRequestException($code);
          case _: throw InternalServerException($code);
        }  }  }

I'd say the problem is that you are most likely inspecting the AST of the sellStock definition, and not finding MyException in there in the second case, so you are not serializing it, right? But you should check all function calls too. This can be very complex; it will not be possible to check all possible code paths.

I think you are overcomplicating this. There is no need to search for exceptions in the AST, but simply create a helper function like the _processErrors below, which checks all possible cloud exceptions exported from exceptions.dart:

void _processErrors(Map<String, Object?> $body, Response $response) {
  final $error = ($body['error'] as Map<String, Object?>);
  final $code = ($error['code'] as String);
  final $details = ($error['details'] as Map<String, Object?>?);
  switch ($code) {
    case r'MyException':
      throw Serializers.instance.deserialize<MyException>($details);
    case r'BadRequestException':
      throw Serializers.instance.deserialize<BadRequestException>($details);
    case r'InternalServerException':
      throw Serializers.instance
          .deserialize<InternalServerException>($details);
    case _:
      switch ($response.statusCode) {
        case 400:
          throw BadRequestException($code);
        case _:
          throw InternalServerException($code);
      }  }  }

And then use that, for all functions:

  Future<({CashBalance cashBalance, Stock stock})> sellStock(
    AvailableStock availableStock, {
    required int howMany,
  }) async {
    final $response = await celest.httpClient.post(
      celest.baseUri.resolve('/portfolio/sell-stock'),
      headers: const {'Content-Type': 'application/json; charset=utf-8'},
      body: jsonEncode({
        r'availableStock':
            Serializers.instance.serialize<AvailableStock>(availableStock),
        r'howMany': howMany,
      }),
    );
    final $body = (jsonDecode($response.body) as Map<String, Object?>);
    if ($response.statusCode == 200) {
      return Serializers.instance
          .deserialize<({CashBalance cashBalance, Stock stock})>(
              $body['response']);
    }
    _processErrors($body, $response);
  }

Not to mention that the original code is very repetitive, and removing it will also save memory. When generating code it's easy to remember to apply the DRY principle to the code we actually wrote, but forget to apply it to the generated code itself.

[marcglasberg]

Not important, but if you are curious, this is the code that caught the bug:

  Bdd(feature)
      .scenario('Selling stocks you don’t have.')
      .given('The user has 120 dollars in cash-balance.')
      .and('IBM price is 30 dollars.')
      .and('The user has no IBM stocks.')
      .when('The user sells 1 IBM.')
      .then('We get an error.')
      .and('The user continues to have 0 IBM.')
      .and('The cash-balance continues to be 120 dollars.')
      .run((ctx) async {

    // Given:
    var ibm = AvailableStock('IBM', name: 'IBM corp', currentPrice: 30.00);
    var state = AppState.from(cashBalance: 120.00, availableStocks: [ibm]);
    expect(state.portfolio.stocks, isEmpty); // No IBM.

    var storeTester = StoreTester(initialState: state);
    await celest.functions.admin.setDatabase(state.portfolio, state.availableStocks.list);

    // When:
    var info = await storeTester.dispatchAndWait(
      SellStock_Action(ibm, howMany: 1),
    );

    // Then:
    expect(info.error, isAError<MyException>('Cannot sell stock you do not own'));
    expect(info.state.portfolio.howManyStocks(ibm.ticker), 0);
    expect(info.state.portfolio.cashBalance, CashBalance(120.00));
  });

And the console output is:

TEST 1  ══════════════════════════════════════════════════

Feature: Buying and Selling Stocks

  Scenario: Selling stocks you don’t have.

    Given The user has 120 dollars in cash-balance.
    And IBM price is 30 dollars.
    And The user has no IBM stocks.

    When The user sells 1 IBM.

    Then We get an error.
    And The user continues to have 0 IBM.
    And The cash-balance continues to be 120 dollars.

══╡ EXCEPTION CAUGHT BY BDD FRAMEWORK ╞═══
The following TestFailure was thrown:
Expected: Error of type "MyException" that contains this text in its message: "Cannot sell stock you
do not own".
  Actual: BadRequestException:<BadRequestException{message: MyException}>
   Which: Expected error of type 'MyException' but threw a 'BadRequestException'.

[dnys1]

Great guess! You're absolutely right that's how I handle it 😆

I had done AST probing before landing on exceptions.dart. Totally agree that is the better solution now 👍

Thanks for bringing this up!

[dnys1]

Tracking the DRY'ing of the exception handling in a separate issue: https://github.com/celest-dev/celest/issues/49

[dnys1]

This has been fixed in 0.2.0!