fix: The patch makes tryCatch invalid

[LinXunFeng]

App ID: N/A

Description

In version 1.0, the tryCatch in the following code will become invalid after the patch is applied.

List<String> list = ['a', 'b'];
bool _isExist(String type) {
  try {
    final model = list.firstWhere(
      (element) => element.type == type,
    );
    return true;
  } catch (e) {
    return false;
  }
}

_isExist('c');

String value = '';
...
try {
  return json.decode(value);
} catch (e) {
  return null;
}

I'm glad this issue has been fixed in version 1.0.1, but I found that the following code still makes tryCatch invalid, even in the latest version 1.0.4.

class HexColor extends Color {
  static int _getColorFromHex(String hexColor) {
    try {
      hexColor = hexColor.toUpperCase().replaceAll("#", "");
      if (hexColor.length == 6) {
        hexColor = "FF$hexColor";
      }
      return int.parse(hexColor, radix: 16);
    } catch (e) {
      return 0x00000000;
    }
  }

  HexColor(final String hexColor) : super(_getColorFromHex(hexColor));
}

HexColor('');

The strange thing is that I new a demo and used the above code. I wanted to reproduce this problem but couldn't. It can only be reproduced in our commercial project.

This issue prevents our page from displaying properly, and it only appears on iOS.

Expected Behavior

The page can be displayed properly.

Additional Context

shorebird --version
Shorebird 1.0.4 • git@github.com:shorebirdtech/shorebird.git
Flutter 3.19.6 • revision aee5222c0d52012f089564a8395826d56e7c2fe8
Engine • revision e4434d7a212b29a8ee8ffaf8e2c453c753904933

[bryanoltman]

This is concerning. Without being able to reproduce the issue, it may be difficult for us to find the cause of the problem, but we will certainly try. Some questions for you:

1. When you say that the try/catch is "invalid", what exactly do you mean?
2. "This issue prevents our page from displaying properly" How? What do you expect to happen? What happens instead?
3. Would you be able to add logs to the code that has this issue to better understand what is going wrong? Without understanding what you're seeing on your end, it's tough to say exactly what these will look like, but being able to trace through the code and pinpoint exactly where it starts to do something we don't expect would be useful.

[bryanoltman]

Reopening, closed by accident

[eseidel]

@felangel and I did have to make a change to the "optimized try catch" logic inside the Dart VM. The compiler typically optimizes away certain local variables in making the "catch" code. It does so by holding onto direct references into the "Object Pool" which we are (later) sorting. We were pretty sure we fixed the one case (in 1.0.1), but it's possible there is a second case?

Would love to work with you to fix. I'm not sure how to reproduce from the code examples you're offering above though?

[LinXunFeng]

For demonstration, I adjusted the above HexColor and the changes are as follows (not important and can be ignored)

class HexColor extends Color {
  static int _getColorFromHex(String hexColor) {
    try {
      hexColor = hexColor.toUpperCase().replaceAll("#", "");
      if (hexColor.length == 6) {
        hexColor = "FF$hexColor";
      }
      return int.parse(hexColor, radix: 16);
    } catch (e) {
-      return 0x00000000;
+      return 0xFF000000;
    }
  }

  HexColor(final String hexColor) : super(_getColorFromHex(hexColor));
}

I added a new test page in our project, the code is as follows

Scaffold(
  backgroundColor: Colors.white,
  appBar: AppBar(
    backgroundColor: Color(0XFFFF8000),
    title: Text(
      'About us',
      style: TextStyle(
        fontSize: 18,
        color: Colors.white,
      ),
    ),
    centerTitle: true,
  ),
  body: Center(
    child: Text(
      'Hello world',
      style: TextStyle(
        color: HexColor(''), // This code will cause an exception after patching
        fontSize: 20,
      ),
    ),
  ),
);

There is no error output on the console at this time.

Adjust the title color and then create the patch.

Scaffold(
  backgroundColor: Colors.white,
  appBar: AppBar(
    backgroundColor: Color(0XFFFF8000),
    title: Text(
      'About us',
      style: TextStyle(
        fontSize: 18,
-        color: Colors.white,
+        color: Colors.purple,
      ),
    ),
    centerTitle: true,
  ),
  body: Center(
    child: Text(
      'Hello world',
      style: TextStyle(
        color: HexColor(''), // This code will cause an exception after patching
        fontSize: 20,
      ),
    ),
  ),
);

[updater::network] Sending patch check request: PatchCheckRequest { app_id: "6fc7bf36-32b9-42f8-b79d-b7c7037a3627", channel: "stable", release_version: "7.1.5+1", patch_number: None, platform: "ios", arch: "aarch64" }
[updater::updater] Patch 1 successfully installed.
[updater::c_api] Update result: Update installed

This app_id belongs to my test account, and the patch is still there. It may be useful to you.

After installing the patch, restart the app and enter the About us page again.

The About us page show grey screen. The following is the error log output by the console.

flutter: Another exception was thrown: Instance of 'DiagnosticsProperty<void>'

The error log this time is different from what I encountered before. Maybe it’s because I didn’t change the same page.

The error I encountered before was this

FormatException: Invalid radix-16 number`.

It was caused by int.parse(hexColor, radix: 16);, so I said tryCatch became invalid.

In addition, this time, my home page also displays a gray screen again, with the following error:

flutter: Bad state: No element
flutter: #0      ListBase.firstWhere (dart:collection/list.dart:124)
flutter: #1      HomeLogicMain._isExistMainModelType
flutter: #2      HomeLogicMain._updateMainEntryByType
...

This is the same problem as before when using version 1.0.

[eseidel]

Thank you, this is very helpful. We will investigate.

[sakhnyuk]

Same issue here. Only on iOS.

In catch block I show toasts or sometimes popup modal with information. After I applied patch in some cases catch block was completely skipped.

[eseidel]

Thank you very much. We just need to get a reproduction (which we can try to do) and then it should be easy to fix.

[fiercepark]

@eseidel If this issue is fixed later, will I still need to release a new app version? Or is it based on the new shorebird generated patch and run on the old app version?

[felangel]

@eseidel If this issue is fixed later, will I still need to release a new app version? Or is it based on the new shorebird generated patch and run on the old app version?

You’ll need to re-release as we’ve made some modifications to the snapshot format as part of #1892