Skip to content

fix: add error handling for launchUrl in About Us feedback link#3375

Closed
prajakta128 wants to merge 4 commits into
fossasia:mainfrom
prajakta128:fix/about-us-launchurl-crash
Closed

fix: add error handling for launchUrl in About Us feedback link#3375
prajakta128 wants to merge 4 commits into
fossasia:mainfrom
prajakta128:fix/about-us-launchurl-crash

Conversation

@prajakta128

Copy link
Copy Markdown
Contributor

Fixes#3374

Problem

The feedback/bug-report link in the About Us screen called launchUrl() directly with no canLaunchUrl check and no try/catch. If the URL fails to launch for any reason (no browser, network issue, invalid URL), the app throws an unhandled exception and crashes.

Root Cause

Line 138 of lib/view/about_us_screen.dart:

onTap: () async {
  await launchUrl(Uri.parse(appLocalizations.feedbackForm));
},

Other links in the same file already use canLaunchUrl guard correctly, but this one was missed.

Fix

Added canLaunchUrl check before calling launchUrl, consistent with the existing pattern used by other links in the same file:

onTap: () async {
  final uri = Uri.parse(appLocalizations.feedbackForm);
  if (await canLaunchUrl(uri)) {
    await launchUrl(uri);
  } else {
    debugPrint('Could not launch ${appLocalizations.feedbackForm}');
  }
},

Impact

  • Prevents app crash when feedback URL cannot be launched
  • Consistent with how all other links in about_us_screen.dart are handled
  • No UI change

Screenshots / Recordings

N/A — crash fix, no UI changes

Checklist

  • No hard coding: I have used values from constants.dart or localization files instead of hard-coded values.
  • No end of file edits: No modifications done at end of resource files.
  • Code reformatting: I have formatted the code using dart format or the IDE formatter.
  • Code analysis: My code passes checks run in flutter analyze and tests run in flutter test.

UsbSerial.usbEventStream?.listen() and Connectivity()
.onConnectivityChanged.listen() were called without storing the
returned StreamSubscription objects, so they could never be
cancelled. This causes a memory leak on every hot-restart or
re-initialization as subscriptions accumulate.

Fix: store both subscriptions in _usbSubscription and
_connectivitySubscription fields, and cancel them in dispose().
The latest Visual Studio removed support for experimental C++ features
(/await compiler option, <experimental/coroutine>), causing the Windows
Flutter build to fail with error STL1011.

Adding _SILENCE_EXPERIMENTAL_COROUTINE_DEPRECATION_WARNINGS to the
existing add_definitions call suppresses the error so Windows builds
work again while a permanent upstream fix is awaited in the
permission_handler_windows plugin.

Fixes fossasia#3246
The feedback link in about_us_screen.dart called launchUrl()
with no canLaunchUrl check or try/catch block. If the URL
fails to launch, this throws an unhandled exception and
crashes the app.

Added canLaunchUrl check consistent with how other links
in the same file are already handled.

Fixes #XXXX

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @prajakta128, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions

Copy link
Copy Markdown
Contributor

Build Status

Build successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/28354945593/artifacts/7945123360.

Screenshots

Android Screenshots
iPhone Screenshots
iPad Screenshots

@rahul31124 rahul31124 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prajakta128, could you please remove the following files?

  • lib/providers/board_state_provider.dart
  • windows/CMakeLists.txt

@prajakta128

Copy link
Copy Markdown
Contributor Author

Closing this PR — it accidentally included files from already-merged PRs (lib/providers/board_state_provider.dart and windows/CMakeLists.txt) due to branching on top of earlier work instead of from main.

I've opened a clean PR with only the About Us fix: #3382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants