Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"on Exception" Doesn't Handle TypeError in External Data Validation #60024

Closed
behzodfaiziev opened this issue Feb 1, 2025 · 3 comments
Closed
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-as-intended Closed as the reported issue is expected behavior

Comments

@behzodfaiziev
Copy link

Dart SDK Version: 3.6.1
Flutter Version: 3.27.3

Problem description:

I am trying to follow the avoid_catches_without_on_clauses linter rule by using on Exception for error handling. However, I’ve encountered an issue where TypeError (a subclass of Error) isn’t caught when processing invalid external data (e.g., API responses). The linter flag for using catch (e) forces me to use on Exception, but this doesn’t catch TypeError, leading to test failures when dealing with invalid data types in API responses.

Expected Behavior:

  • on Exception should catch errors like TypeError, which occur during external data validation.

Actual Behavior:

  • on Exception does not catch TypeError, causing the test to fail when an invalid type is encountered.

Steps to reproduce: First Option

Test Code: Line 152

test('should return null tokens when tokens are not strings', () {  
  final response = Response<dynamic>(  
    requestOptions: RequestOptions(path: '/test'),  
    data: <String, dynamic>{  
      'accessToken': 12345, // int (invalid)  
      'refreshToken': true, // bool (invalid)  
    },  
  );  

  final tokens = netKitManager.extractTokens(response: response);  
  expect(tokens.accessToken, isNull);  
  expect(tokens.refreshToken, isNull);  
});

Implementation Code: Line 29

  AuthTokenModel extractTokens({required Response<dynamic> response}) {
    try {
      if ((response.statusCode ?? 0) >= HttpStatus.internalServerError) {
        return const AuthTokenModel();
      }

      // Ensure response.data is treated as a Map
      final data = response.data as Map<String, dynamic>?;

      if (data == null) {
        return const AuthTokenModel();
      }

      return AuthTokenModel(
        accessToken: data[parameters.accessTokenBodyKey] as String?, // Throws TypeError  
        refreshToken: data[parameters.refreshTokenBodyKey] as String?,
      );
    } on Exception catch (_) {
      return const AuthTokenModel(); // catch should have been executed on errro
    }
  }

Steps to Reproduce: Second option

  1. Clone the repository NetKit
  2. After getting the packages run:
cd packages/net-kit && dart test
  1. see error:
'NetKitManager Extract tokens from body should return null tokens when tokens are not strings'
00:04 +57 -1: Some tests failed.

Behavior

With on Exception:

❌ Test fails (TypeError propagates and isn't caught by on Exception).

With catch (e) (violates the linter rule):

⚠️ Test passes (tokens are null), but the linter warns about using catch (e).

@behzodfaiziev behzodfaiziev added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Feb 1, 2025
@RohitSaily
Copy link
Contributor

I believe TypeError is a subclass of Error which is not a subclass of Exception. If you intend to keep the lint rule active but want a catch-all clause, use on Object instead. Throw expressions are required to always evaluate to Object so this clause will catch anything, whether it is an Exception, Error, or anything else.

@behzodfaiziev
Copy link
Author

behzodfaiziev commented Feb 1, 2025

@RohitSaily Thank you for recommending Object. It works well because it catches both Exception and Error, including TypeError. But this seems more like a workaround than an intended best practice. Would it make sense to include this use case in the Dart documentation? For example, when handling API responses or dynamic data, using on Object might actually be the right approach. Should the linter rule (avoid_catches_without_on_clauses) acknowledge cases where on Object is the best option?

@lrhn
Copy link
Member

lrhn commented Feb 1, 2025

This is working as intended.

The type Exception is intended as a supertype of exceptions: exceptional, but expected, outcomes of an operation, usually not under the caller's control. Exceptions are intended to be caught by the caller, as if they were a different kind of return.
You should always throw a subtype of Exception, document that it's thrown, and the caller should always catch the expected subtype of Exception (never throw Exception("message") and never } on Exception catch (e) {).

The type Error is intended as a the supertype of thrown errors. A correct Dart program should never throw an Error. The caller should never catch an Error (no on Error catch (e) or fx on AssertionError catch (e)). If it's framework wrapper that catches all thrown objects, it should use on Object.

The two types are intended for mutually exclusive situations, and they are not related.

@lrhn lrhn closed this as not planned Won't fix, can't repro, duplicate, stale Feb 1, 2025
@lrhn lrhn added the closed-as-intended Closed as the reported issue is expected behavior label Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

3 participants