fix: add error handling for launchUrl in About Us feedback link#3382
Merged
marcnause merged 3 commits intoJul 1, 2026
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds proper error handling around the About Us feedback URL tap handler by guarding launchUrl with canLaunchUrl and logging failures, and performs a small formatting cleanup in version-info error logging. Sequence diagram for updated About Us feedback link error handlingsequenceDiagram
actor User
participant AboutUsScreen
participant UrlLauncher
participant Logger
User ->> AboutUsScreen: onTap
AboutUsScreen ->> UrlLauncher: canLaunchUrl(uri)
UrlLauncher -->> AboutUsScreen: bool
alt [canLaunchUrl is true]
AboutUsScreen ->> UrlLauncher: launchUrl(uri)
else [canLaunchUrl is false]
AboutUsScreen ->> Logger: debugPrint
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Instead of using
debugPrintfor the failure case, consider routing this through the existingloggerto keep logging behavior consistent with the rest of the screen. - Right now a failure to open the feedback URL is only logged; consider providing a small user-visible indication (e.g., a Snackbar) so users understand why nothing happened when they tapped the link.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of using `debugPrint` for the failure case, consider routing this through the existing `logger` to keep logging behavior consistent with the rest of the screen.
- Right now a failure to open the feedback URL is only logged; consider providing a small user-visible indication (e.g., a Snackbar) so users understand why nothing happened when they tapped the link.
## Individual Comments
### Comment 1
<location path="lib/view/about_us_screen.dart" line_range="138" />
<code_context>
),
onTap: () async {
- await launchUrl(Uri.parse(appLocalizations.feedbackForm));
+ final uri = Uri.parse(appLocalizations.feedbackForm);
+ if (await canLaunchUrl(uri)) {
+ await launchUrl(uri);
</code_context>
<issue_to_address>
**suggestion:** Consider using the existing logger and/or surfacing a user-visible error instead of debugPrint.
`debugPrint` limits visibility of failures to debug builds only. Since this is user-facing and you’re already using `logger` in this file, please log via `logger` instead and consider lightweight UI feedback (e.g. a snackbar) so users are aware when opening the feedback form fails.
Suggested implementation:
```
final uri = Uri.parse(appLocalizations.feedbackForm);
if (await canLaunchUrl(uri)) {
await launchUrl(uri);
} else {
logger.e(
'Could not launch feedback form URL',
'url=${appLocalizations.feedbackForm}',
);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
appLocalizations.feedbackFormOpenError,
),
),
);
}
```
1. Ensure `appLocalizations.feedbackFormOpenError` (or equivalent) exists; if not, add a localized string for the snackbar message.
2. If your project uses a custom snackbar helper instead of `ScaffoldMessenger.of`, replace that call with the appropriate helper while keeping the `logger.e` call.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4 tasks
The feedback/bug-report link in about_us_screen.dart called launchUrl() directly with no canLaunchUrl check and no try/catch. If the URL fails to launch for any reason, the app throws an unhandled exception and crashes. Added canLaunchUrl check before calling launchUrl, consistent with the existing pattern used by other links in the same file. Fixes fossasia#3374
Per code review feedback, replaced debugPrint with the project's existing logger service to keep logging consistent with the rest of the app.
The buildContactList onTap was accidentally pointing to the feedback form URL instead of each contact item's own URL. Also fixed logger.e calls to use a single string argument, matching the pattern used elsewhere in the file, and restored the original version-error message.
e6af0fc to
2ec43dc
Compare
marcnause
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3374
Problem
The feedback/bug-report link in the About Us screen called
launchUrl()directly with nocanLaunchUrlcheck and notry/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.dartcalled launchUrl directly without checking if the URL could actually be launched. Other links in the same file already use acanLaunchUrlguard correctly, but this one was missed.Fix
Added a
canLaunchUrlcheck before callinglaunchUrl, consistent with the existing pattern used by other links in the same file. If the URL cannot be launched, it now logs a debug message instead of crashing.Impact
Screenshots / Recordings
N/A — crash fix, no UI changes
Checklist
constants.dartor localization files instead of hard-coded values.dart formator the IDE formatter.flutter analyzeand tests run influtter test.Summary by Sourcery
Add defensive URL launch handling for the About Us feedback link to prevent crashes when the feedback form cannot be opened.
Bug Fixes:
Enhancements: