Fix build crash when key.properties is missing and add example config#802
Open
Shweta-281 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Open
Fix build crash when key.properties is missing and add example config#802Shweta-281 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Shweta-281 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Conversation
Contributor
|
🎉 Welcome @Shweta-281!
We appreciate your contribution! 🚀 |
📝 WalkthroughWalkthroughThis PR establishes cross-platform native project infrastructure for a Flutter application. It adds build configurations, native source files, and platform-specific setup for iOS, Linux, macOS, and Windows, resolves Android keystore handling issues, and updates project metadata and dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/build.gradle.kts (1)
50-63:⚠️ Potential issue | 🟠 MajorRelease signing config override negates fallback to debug, causing failures when
key.propertiesis missing.Lines 61 and 62 both assign
signingConfig: Line 61 sets it to"debug"as intended fallback, but Line 62 unconditionally overwrites it with"release". Combined with empty-string credentials on Lines 50/51/53 (when keystore properties are absent), the release config will be applied with invalid credentials, breaking the build.Suggested fix
+ val hasReleaseKeystore = listOf("keyAlias", "keyPassword", "storeFile", "storePassword") + .all { !keystoreProperties.getProperty(it).isNullOrBlank() } + buildTypes { signingConfigs { create("release") { - keyAlias = keystoreProperties["keyAlias"] as String? ?: "" - keyPassword = keystoreProperties["keyPassword"] as String? ?: "" - storeFile = keystoreProperties["storeFile"]?.let { file(it) } - storePassword = keystoreProperties["storePassword"] as String? ?: "" + keyAlias = keystoreProperties.getProperty("keyAlias") + keyPassword = keystoreProperties.getProperty("keyPassword") + storeFile = keystoreProperties.getProperty("storeFile")?.let { file(it) } + storePassword = keystoreProperties.getProperty("storePassword") } } release { isMinifyEnabled = false isShrinkResources = false - signingConfig = signingConfigs.getByName("debug") - signingConfig = signingConfigs.getByName("release") + signingConfig = if (hasReleaseKeystore) { + signingConfigs.getByName("release") + } else { + signingConfigs.getByName("debug") + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/build.gradle.kts` around lines 50 - 63, The release build block currently sets signingConfig twice (signingConfig = signingConfigs.getByName("debug") then signingConfig = signingConfigs.getByName("release")) which overwrites the intended debug fallback and causes invalid release signing when keystoreProperties (keyAlias, keyPassword, storeFile, storePassword) are missing; modify the release block so signingConfig is only set to the debug signingConfig by default and only switched to the release signingConfig when valid keystoreProperties exist (e.g., check storeFile/keyAlias or use signingConfigs.findByName("release") conditionally), ensuring signingConfig is not unconditionally reassigned to "release".
🧹 Nitpick comments (9)
web/manifest.json (1)
8-8: Customize the generic description.Similar to
index.html, the manifest description "A new Flutter project." is boilerplate that should be replaced with a meaningful description of the Resonate app.📝 Suggested improvement
- "description": "A new Flutter project.", + "description": "Resonate - A music streaming and social audio platform.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/manifest.json` at line 8, Replace the boilerplate description value in the web/manifest.json "description" property ("A new Flutter project.") with a meaningful, app-specific description for Resonate; locate the "description" field in web/manifest.json and update its string to succinctly describe Resonate's purpose and core functionality.web/index.html (1)
21-21: Customize the generic description.The description "A new Flutter project." is boilerplate text that should be replaced with an accurate description of the Resonate app.
📝 Suggested improvement
- <meta name="description" content="A new Flutter project."> + <meta name="description" content="Resonate - A music streaming and social audio platform.">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/index.html` at line 21, Replace the boilerplate meta description value in the HTML meta tag with a concise, accurate summary of the Resonate app by updating the <meta name="description"> content attribute to describe Resonate's purpose, key features, and target audience (e.g., collaborative music discovery, personalized playlists, social sharing) so the page no longer uses "A new Flutter project." and reflects the app's actual functionality.linux/.gitignore (1)
1-1: Minor inconsistency: consider adding trailing slash for directory pattern.Other platform
.gitignorefiles in this PR (e.g.,windows/.gitignore,macos/.gitignore) use trailing slashes for directory patterns (flutter/ephemeral/). Consider adding the trailing slash here for consistency.Suggested change
-flutter/ephemeral +flutter/ephemeral/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@linux/.gitignore` at line 1, The gitignore entry "flutter/ephemeral" should be made consistent with other platform files by adding a trailing slash to indicate a directory; update the entry in linux/.gitignore from "flutter/ephemeral" to "flutter/ephemeral/" so it matches the style used in windows/.gitignore and macos/.gitignore.android/key.properties.example (1)
1-4: Good addition that directly addresses the PR objective.This example file guides contributors on keystore setup. Consider adding brief comments to clarify path expectations.
📝 Suggested improvement with comments
+# Android keystore configuration for release signing +# Copy this file to key.properties and fill in your values storePassword=your_password keyPassword=your_password keyAlias=upload -storeFile=path_to_keystore +# Path to your keystore file (relative to android/app/ or absolute) +storeFile=../your-keystore.jks🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/key.properties.example` around lines 1 - 4, Add short inline comments to the key.properties.example file explaining what each property represents and how to set the storeFile path (e.g., relative vs absolute path and typical keystore filenames). Specifically annotate the properties storePassword, keyPassword, keyAlias, and storeFile so contributors know which values to replace and how to reference the keystore (relative path under android/ or absolute path), and mention any common default alias like "upload".macos/RunnerTests/RunnerTests.swift (1)
7-10: Replace placeholder test with a real assertion-based check.Right now this test provides no validation and can give false confidence in CI results.
Suggested direction
class RunnerTests: XCTestCase { func testExample() { - // If you add code to the Runner application, consider adding tests here. - // See https://developer.apple.com/documentation/xctest for more information about using XCTest. + // Add at least one smoke assertion for current macOS runner behavior. } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@macos/RunnerTests/RunnerTests.swift` around lines 7 - 10, The placeholder testExample in the RunnerTests test case contains no assertions—replace it with a real assertion-based check to ensure CI fails on regressions; update the testExample method (or add a new test in the RunnerTests class) to perform a concrete XCTAssert call (e.g., XCTAssertNotNil/XCTAssertTrue/XCTAssertEqual depending on what you want to validate) against a deterministic value or runtime property (bundle identifier, known computation, or a simple sanity check) so the test actually verifies behavior rather than being empty.windows/runner/utils.cpp (1)
48-56: Potential unsigned integer wraparound whenWideCharToMultiBytefails.If
WideCharToMultiBytefails and returns0, subtracting1from theunsigned int target_lengthcauses wraparound toUINT_MAX. While the subsequentmax_size()check catches this, the logic is fragile and unclear.♻️ Suggested improvement for clarity
- unsigned int target_length = ::WideCharToMultiByte( + int target_length = ::WideCharToMultiByte( CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string, - -1, nullptr, 0, nullptr, nullptr) - -1; // remove the trailing null character - int input_length = (int)wcslen(utf16_string); + -1, nullptr, 0, nullptr, nullptr); + if (target_length <= 1) { + return std::string(); + } + target_length -= 1; // remove the trailing null character std::string utf8_string; - if (target_length == 0 || target_length > utf8_string.max_size()) { + if (static_cast<size_t>(target_length) > utf8_string.max_size()) { return utf8_string; } + int input_length = static_cast<int>(wcslen(utf16_string)); utf8_string.resize(target_length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@windows/runner/utils.cpp` around lines 48 - 56, Avoid unsigned wraparound by checking WideCharToMultiByte's return before subtracting: call ::WideCharToMultiByte and store its result in a signed/size_t temporary (e.g., raw_target_len), if raw_target_len == 0 return empty utf8_string (or handle the error), else compute target_length = raw_target_len - 1; update references to target_length and keep the existing max_size() guard. Reference symbols: ::WideCharToMultiByte, utf16_string, target_length, raw_target_len (new), and utf8_string.windows/runner/win32_window.cpp (2)
221-221: Usingwindow_handle_instead ofhwndparameter.At line 221,
DefWindowProcis called withwindow_handle_instead of thehwndparameter passed toMessageHandler. While these should typically be the same, using the parameter directly is more consistent and avoids potential issues if they ever differ (e.g., during destruction whenwindow_handle_may be null).♻️ Suggested fix
- return DefWindowProc(window_handle_, message, wparam, lparam); + return DefWindowProc(hwnd, message, wparam, lparam);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@windows/runner/win32_window.cpp` at line 221, In MessageHandler, replace the use of the member variable window_handle_ when calling DefWindowProc with the incoming hwnd parameter so the call becomes DefWindowProc(hwnd, message, wparam, lparam); locate the call at the end of MessageHandler and update the argument list to use hwnd instead of window_handle_.
89-107: Missing error handling forRegisterClass.
RegisterClasscan fail (returning 0), but the return value is not checked. While unlikely to fail in practice, checking the result would make the code more robust.🛡️ Suggested defensive check
const wchar_t* WindowClassRegistrar::GetWindowClass() { if (!class_registered_) { WNDCLASS window_class{}; window_class.hCursor = LoadCursor(nullptr, IDC_ARROW); window_class.lpszClassName = kWindowClassName; window_class.style = CS_HREDRAW | CS_VREDRAW; window_class.cbClsExtra = 0; window_class.cbWndExtra = 0; window_class.hInstance = GetModuleHandle(nullptr); window_class.hIcon = LoadIcon(window_class.hInstance, MAKEINTRESOURCE(IDI_APP_ICON)); window_class.hbrBackground = 0; window_class.lpszMenuName = nullptr; window_class.lpfnWndProc = Win32Window::WndProc; - RegisterClass(&window_class); - class_registered_ = true; + if (RegisterClass(&window_class)) { + class_registered_ = true; + } } return kWindowClassName; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@windows/runner/win32_window.cpp` around lines 89 - 107, The RegisterClass call in WindowClassRegistrar::GetWindowClass is unchecked; update the function to verify RegisterClass (or RegisterClassW) returns nonzero, only set class_registered_ on success, and on failure retrieve the OS error via GetLastError and handle it (e.g., log the error with context and return nullptr or throw an exception) so callers don't assume the class was registered when it wasn't.windows/runner/flutter_window.cpp (1)
43-45: Redundant null check before assignment.The conditional check is unnecessary when assigning
nullptr- the assignment is safe regardless.♻️ Suggested simplification
void FlutterWindow::OnDestroy() { - if (flutter_controller_) { - flutter_controller_ = nullptr; - } + flutter_controller_ = nullptr; Win32Window::OnDestroy(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@windows/runner/flutter_window.cpp` around lines 43 - 45, Remove the redundant conditional around resetting the flutter_controller_ member: always assign flutter_controller_ = nullptr without checking its current value (replace the if (flutter_controller_) { flutter_controller_ = nullptr; } pattern with a direct assignment to flutter_controller_). This simplifies the code and avoids an unnecessary null check while preserving the intended behavior of the destructor/cleanup path that touches flutter_controller_.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/widget_test.dart`:
- Around line 13-29: The current widget_test.dart contains the Flutter template
boilerplate that asserts a Counter UI which doesn't exist in your app; replace
the failing test by removing the Counter-specific expectations and the Icons.add
interaction in the testWidgets block and instead write tests that exercise
MyApp/GetMaterialApp startup and real app behaviors (e.g., verify that MyApp
builds without exceptions, that GetMaterialApp shows expected localized strings,
theme elements, or key widgets for the Resonate UI). Locate the test function
named testWidgets in widget_test.dart and update its assertions to look for
actual widgets/text/keys present in MyApp (or add new tests that pump MyApp and
assert presence of localization keys, navigation routes, or specific widget keys
used by the app).
In `@windows/runner/flutter_window.cpp`:
- Around line 64-68: The WM_FONTCHANGE case in
FlutterWindow::HandleTopLevelWindowProc calls flutter_controller_->engine()
without ensuring flutter_controller_ is non-null; update the WM_FONTCHANGE
handler to first check flutter_controller_ (and/or
flutter_controller_->engine()) before calling ReloadSystemFonts(),
returning/ignoring the message if the controller or engine is null. Locate the
switch handling message in HandleTopLevelWindowProc and add a null guard around
the flutter_controller_->engine()->ReloadSystemFonts() invocation to prevent a
potential null pointer dereference.
In `@windows/runner/utils.cpp`:
- Around line 10-22: The error check for freopen_s is inverted in
CreateAndAttachConsole: freopen_s returns 0 on success, but the code calls _dup2
only when freopen_s returns non-zero; change the condition so that
_dup2(_fileno(stdout), 1) runs when freopen_s(&unused, "CONOUT$", "w", stdout)
returns 0, and likewise _dup2(_fileno(stdout), 2) runs when freopen_s(&unused,
"CONOUT$", "w", stderr) returns 0; keep the rest of the function (AllocConsole,
std::ios::sync_with_stdio(), FlutterDesktopResyncOutputStreams()) unchanged.
---
Outside diff comments:
In `@android/app/build.gradle.kts`:
- Around line 50-63: The release build block currently sets signingConfig twice
(signingConfig = signingConfigs.getByName("debug") then signingConfig =
signingConfigs.getByName("release")) which overwrites the intended debug
fallback and causes invalid release signing when keystoreProperties (keyAlias,
keyPassword, storeFile, storePassword) are missing; modify the release block so
signingConfig is only set to the debug signingConfig by default and only
switched to the release signingConfig when valid keystoreProperties exist (e.g.,
check storeFile/keyAlias or use signingConfigs.findByName("release")
conditionally), ensuring signingConfig is not unconditionally reassigned to
"release".
---
Nitpick comments:
In `@android/key.properties.example`:
- Around line 1-4: Add short inline comments to the key.properties.example file
explaining what each property represents and how to set the storeFile path
(e.g., relative vs absolute path and typical keystore filenames). Specifically
annotate the properties storePassword, keyPassword, keyAlias, and storeFile so
contributors know which values to replace and how to reference the keystore
(relative path under android/ or absolute path), and mention any common default
alias like "upload".
In `@linux/.gitignore`:
- Line 1: The gitignore entry "flutter/ephemeral" should be made consistent with
other platform files by adding a trailing slash to indicate a directory; update
the entry in linux/.gitignore from "flutter/ephemeral" to "flutter/ephemeral/"
so it matches the style used in windows/.gitignore and macos/.gitignore.
In `@macos/RunnerTests/RunnerTests.swift`:
- Around line 7-10: The placeholder testExample in the RunnerTests test case
contains no assertions—replace it with a real assertion-based check to ensure CI
fails on regressions; update the testExample method (or add a new test in the
RunnerTests class) to perform a concrete XCTAssert call (e.g.,
XCTAssertNotNil/XCTAssertTrue/XCTAssertEqual depending on what you want to
validate) against a deterministic value or runtime property (bundle identifier,
known computation, or a simple sanity check) so the test actually verifies
behavior rather than being empty.
In `@web/index.html`:
- Line 21: Replace the boilerplate meta description value in the HTML meta tag
with a concise, accurate summary of the Resonate app by updating the <meta
name="description"> content attribute to describe Resonate's purpose, key
features, and target audience (e.g., collaborative music discovery, personalized
playlists, social sharing) so the page no longer uses "A new Flutter project."
and reflects the app's actual functionality.
In `@web/manifest.json`:
- Line 8: Replace the boilerplate description value in the web/manifest.json
"description" property ("A new Flutter project.") with a meaningful,
app-specific description for Resonate; locate the "description" field in
web/manifest.json and update its string to succinctly describe Resonate's
purpose and core functionality.
In `@windows/runner/flutter_window.cpp`:
- Around line 43-45: Remove the redundant conditional around resetting the
flutter_controller_ member: always assign flutter_controller_ = nullptr without
checking its current value (replace the if (flutter_controller_) {
flutter_controller_ = nullptr; } pattern with a direct assignment to
flutter_controller_). This simplifies the code and avoids an unnecessary null
check while preserving the intended behavior of the destructor/cleanup path that
touches flutter_controller_.
In `@windows/runner/utils.cpp`:
- Around line 48-56: Avoid unsigned wraparound by checking WideCharToMultiByte's
return before subtracting: call ::WideCharToMultiByte and store its result in a
signed/size_t temporary (e.g., raw_target_len), if raw_target_len == 0 return
empty utf8_string (or handle the error), else compute target_length =
raw_target_len - 1; update references to target_length and keep the existing
max_size() guard. Reference symbols: ::WideCharToMultiByte, utf16_string,
target_length, raw_target_len (new), and utf8_string.
In `@windows/runner/win32_window.cpp`:
- Line 221: In MessageHandler, replace the use of the member variable
window_handle_ when calling DefWindowProc with the incoming hwnd parameter so
the call becomes DefWindowProc(hwnd, message, wparam, lparam); locate the call
at the end of MessageHandler and update the argument list to use hwnd instead of
window_handle_.
- Around line 89-107: The RegisterClass call in
WindowClassRegistrar::GetWindowClass is unchecked; update the function to verify
RegisterClass (or RegisterClassW) returns nonzero, only set class_registered_ on
success, and on failure retrieve the OS error via GetLastError and handle it
(e.g., log the error with context and return nullptr or throw an exception) so
callers don't assume the class was registered when it wasn't.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abeebef2-f31e-4e9c-b61e-3cd598e9b390
⛔ Files ignored due to path filters (29)
ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_1024.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_128.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_16.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_256.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_32.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_512.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_64.pngis excluded by!**/*.pngpubspec.lockis excluded by!**/*.lockweb/favicon.pngis excluded by!**/*.pngweb/icons/Icon-192.pngis excluded by!**/*.pngweb/icons/Icon-512.pngis excluded by!**/*.pngweb/icons/Icon-maskable-192.pngis excluded by!**/*.pngweb/icons/Icon-maskable-512.pngis excluded by!**/*.pngwindows/runner/resources/app_icon.icois excluded by!**/*.ico
📒 Files selected for processing (58)
.gitignore.metadataandroid/app/build.gradle.ktsandroid/key.properties.exampleios/Runner/SceneDelegate.swiftios/RunnerTests/RunnerTests.swiftlinux/.gitignorelinux/CMakeLists.txtlinux/flutter/CMakeLists.txtlinux/flutter/generated_plugin_registrant.cclinux/flutter/generated_plugin_registrant.hlinux/flutter/generated_plugins.cmakelinux/runner/CMakeLists.txtlinux/runner/main.cclinux/runner/my_application.cclinux/runner/my_application.hmacos/.gitignoremacos/Flutter/Flutter-Debug.xcconfigmacos/Flutter/Flutter-Release.xcconfigmacos/Flutter/GeneratedPluginRegistrant.swiftmacos/Runner.xcodeproj/project.pbxprojmacos/Runner.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistmacos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschememacos/Runner.xcworkspace/contents.xcworkspacedatamacos/Runner.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistmacos/Runner/AppDelegate.swiftmacos/Runner/Assets.xcassets/AppIcon.appiconset/Contents.jsonmacos/Runner/Base.lproj/MainMenu.xibmacos/Runner/Configs/AppInfo.xcconfigmacos/Runner/Configs/Debug.xcconfigmacos/Runner/Configs/Release.xcconfigmacos/Runner/Configs/Warnings.xcconfigmacos/Runner/DebugProfile.entitlementsmacos/Runner/Info.plistmacos/Runner/MainFlutterWindow.swiftmacos/Runner/Release.entitlementsmacos/RunnerTests/RunnerTests.swiftpubspec.yamltest/widget_test.dartweb/index.htmlweb/manifest.jsonwindows/.gitignorewindows/CMakeLists.txtwindows/flutter/CMakeLists.txtwindows/flutter/generated_plugin_registrant.ccwindows/flutter/generated_plugin_registrant.hwindows/flutter/generated_plugins.cmakewindows/runner/CMakeLists.txtwindows/runner/Runner.rcwindows/runner/flutter_window.cppwindows/runner/flutter_window.hwindows/runner/main.cppwindows/runner/resource.hwindows/runner/runner.exe.manifestwindows/runner/utils.cppwindows/runner/utils.hwindows/runner/win32_window.cppwindows/runner/win32_window.h
Comment on lines
+13
to
+29
| void main() { | ||
| testWidgets('Counter increments smoke test', (WidgetTester tester) async { | ||
| // Build our app and trigger a frame. | ||
| await tester.pumpWidget(const MyApp()); | ||
|
|
||
| // Verify that our counter starts at 0. | ||
| expect(find.text('0'), findsOneWidget); | ||
| expect(find.text('1'), findsNothing); | ||
|
|
||
| // Tap the '+' icon and trigger a frame. | ||
| await tester.tap(find.byIcon(Icons.add)); | ||
| await tester.pump(); | ||
|
|
||
| // Verify that our counter has incremented. | ||
| expect(find.text('0'), findsNothing); | ||
| expect(find.text('1'), findsOneWidget); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if MyApp contains counter-related elements
rg -n "Icons\.add|findsOneWidget.*'0'|Text\('0'\)" --type=dart lib/
# Check what MyApp actually renders
ast-grep --pattern $'class MyApp extends $_ {
$$$
}'Repository: AOSSIE-Org/Resonate
Length of output: 498
🏁 Script executed:
#!/bin/bash
# Find and read the main.dart file to see MyApp definition
find . -name "main.dart" -type f | head -5Repository: AOSSIE-Org/Resonate
Length of output: 78
🏁 Script executed:
#!/bin/bash
# Check if test/widget_test.dart exists and read it
if [ -f "test/widget_test.dart" ]; then
echo "=== test/widget_test.dart exists ==="
cat -n test/widget_test.dart | head -50
else
echo "=== test/widget_test.dart NOT found ==="
find . -name "widget_test.dart" -o -name "*_test.dart" | head -10
fiRepository: AOSSIE-Org/Resonate
Length of output: 1368
🏁 Script executed:
#!/bin/bash
# Check what MyApp class actually looks like in lib/main.dart
rg -A 20 "class MyApp extends" lib/main.dartRepository: AOSSIE-Org/Resonate
Length of output: 746
This test is Flutter template boilerplate and will fail.
The test expects a counter app with text widgets displaying '0' and '1' plus an Icons.add button, but MyApp is a StatelessWidget that returns a GetMaterialApp configured for the Resonate audio streaming app with localization and theming. These expected UI elements do not exist, causing the test to fail. This file should be replaced with tests appropriate for the actual app functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/widget_test.dart` around lines 13 - 29, The current widget_test.dart
contains the Flutter template boilerplate that asserts a Counter UI which
doesn't exist in your app; replace the failing test by removing the
Counter-specific expectations and the Icons.add interaction in the testWidgets
block and instead write tests that exercise MyApp/GetMaterialApp startup and
real app behaviors (e.g., verify that MyApp builds without exceptions, that
GetMaterialApp shows expected localized strings, theme elements, or key widgets
for the Resonate UI). Locate the test function named testWidgets in
widget_test.dart and update its assertions to look for actual widgets/text/keys
present in MyApp (or add new tests that pump MyApp and assert presence of
localization keys, navigation routes, or specific widget keys used by the app).
Comment on lines
+64
to
+68
| switch (message) { | ||
| case WM_FONTCHANGE: | ||
| flutter_controller_->engine()->ReloadSystemFonts(); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Potential null pointer dereference in WM_FONTCHANGE handler.
The WM_FONTCHANGE case accesses flutter_controller_->engine() without checking if flutter_controller_ is non-null. The null check on line 55 only guards HandleTopLevelWindowProc - execution can still reach the switch statement when flutter_controller_ is null.
🐛 Proposed fix
switch (message) {
case WM_FONTCHANGE:
- flutter_controller_->engine()->ReloadSystemFonts();
+ if (flutter_controller_) {
+ flutter_controller_->engine()->ReloadSystemFonts();
+ }
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| switch (message) { | |
| case WM_FONTCHANGE: | |
| flutter_controller_->engine()->ReloadSystemFonts(); | |
| break; | |
| } | |
| switch (message) { | |
| case WM_FONTCHANGE: | |
| if (flutter_controller_) { | |
| flutter_controller_->engine()->ReloadSystemFonts(); | |
| } | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@windows/runner/flutter_window.cpp` around lines 64 - 68, The WM_FONTCHANGE
case in FlutterWindow::HandleTopLevelWindowProc calls
flutter_controller_->engine() without ensuring flutter_controller_ is non-null;
update the WM_FONTCHANGE handler to first check flutter_controller_ (and/or
flutter_controller_->engine()) before calling ReloadSystemFonts(),
returning/ignoring the message if the controller or engine is null. Locate the
switch handling message in HandleTopLevelWindowProc and add a null guard around
the flutter_controller_->engine()->ReloadSystemFonts() invocation to prevent a
potential null pointer dereference.
Comment on lines
+10
to
+22
| void CreateAndAttachConsole() { | ||
| if (::AllocConsole()) { | ||
| FILE *unused; | ||
| if (freopen_s(&unused, "CONOUT$", "w", stdout)) { | ||
| _dup2(_fileno(stdout), 1); | ||
| } | ||
| if (freopen_s(&unused, "CONOUT$", "w", stderr)) { | ||
| _dup2(_fileno(stdout), 2); | ||
| } | ||
| std::ios::sync_with_stdio(); | ||
| FlutterDesktopResyncOutputStreams(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Flutter Windows runner CreateAndAttachConsole freopen_s usage pattern
💡 Result:
In Flutter Windows runner, the CreateAndAttachConsole function in utils.cpp allocates a new console using AllocConsole and redirects stdout/stderr to it using freopen_s with "CONOUT$". The standard usage pattern is: void CreateAndAttachConsole { if (::AllocConsole) { FILE unused; if (freopen_s(&unused, "CONOUT$", "w", stdout)) { _dup2(_fileno(stdout), 1); } if (freopen_s(&unused, "CONOUT$", "w", stderr)) { _dup2(_fileno(stdout), 2); } std::ios::sync_with_stdio; FlutterDesktopResyncOutputStreams; } } This pattern first calls AllocConsole to create the console if needed. Then freopen_s is used twice: once for stdout and once for stderr, redirecting both to "CONOUT$" in write mode. A dummy FILE pointer 'unused' captures the new stream handle. If freopen_s succeeds, _dup2 duplicates the file descriptor to the standard handles (1 for stdout, 2 for stderr) to ensure proper inheritance. Finally, std::ios::sync_with_stdio syncs C++ iostreams with C stdio, and FlutterDesktopResyncOutputStreams resyncs Flutter engine streams. This is called conditionally in main.cpp, typically when no parent console exists and a debugger is present: if (!::AttachConsole(ATTACH_PARENT_PROCESS) && ::IsDebuggerPresent) { CreateAndAttachConsole; } The pattern ensures console output works during debugging or when explicitly needed, common in Flutter Windows desktop apps built as GUI (SUBSYSTEM:WINDOWS) without automatic console.
Citations:
- 1: https://flutter.googlesource.com/mirrors/flutter/+/refs/heads/flutter-3.4-candidate.11/examples/api/windows/runner/utils.cpp
- 2: https://chromium.googlesource.com/external/github.com/flutter/packages/+/842e2db2fba91695da2a2354b4ff3f2382fad4b5/packages/platform/example/windows/runner/utils.cpp
- 3: https://chromium.googlesource.com/external/github.com/flutter/flutter/+/refs/tags/3.17.0-18.0.pre/examples/platform_channel/windows/runner/utils.cpp
- 4: Hiding process console with no parent console to inherit from dart-lang/sdk#39945
- 5: No console output when debugging in Visual Studio 2019 flutter/flutter#53618
🏁 Script executed:
find . -name "utils.cpp" -path "*/windows/runner/*" | head -5Repository: AOSSIE-Org/Resonate
Length of output: 89
🏁 Script executed:
# Once we locate the file, read the relevant lines
fd -e cpp -p "*windows/runner*" | grep utilsRepository: AOSSIE-Org/Resonate
Length of output: 358
🏁 Script executed:
cat -n ./windows/runner/utils.cpp | head -30Repository: AOSSIE-Org/Resonate
Length of output: 1023
🏁 Script executed:
# Also check Microsoft documentation about freopen_s return value
web_search
Microsoft freopen_s return value errno_t documentationRepository: AOSSIE-Org/Resonate
Length of output: 161
🌐 Web query:
Microsoft freopen_s return value errno_t documentation
💡 Result:
The Microsoft freopen_s function returns an errno_t value: zero (0) on success, or a non-zero error code on failure. If an error occurs, the original file associated with oldStream is closed, and NULL is written to *stream (unless stream is also NULL). For details on specific error codes, refer to errno, _doserrno, _sys_errlist, and _sys_nerr. Syntax: errno_t freopen_s(FILE stream, const char *fileName, const char *mode, FILE *oldStream); Example usage checks if(err != 0) to detect failure.
Citations:
- 1: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/freopen-s-wfreopen-s?view=msvc-170
- 2: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/freopen-s-wfreopen-s?view=msvc-170&viewFallbackFrom=vs-2019
- 3: https://learn.microsoft.com/en-us/previous-versions/88k7d7a7(v=vs.140)
Inverted error check for freopen_s.
freopen_s returns 0 on success and a non-zero error code on failure. The current logic executes _dup2 only when freopen_s fails, which is the opposite of the intended behavior.
🐛 Proposed fix
void CreateAndAttachConsole() {
if (::AllocConsole()) {
FILE *unused;
- if (freopen_s(&unused, "CONOUT$", "w", stdout)) {
+ if (freopen_s(&unused, "CONOUT$", "w", stdout) == 0) {
_dup2(_fileno(stdout), 1);
}
- if (freopen_s(&unused, "CONOUT$", "w", stderr)) {
+ if (freopen_s(&unused, "CONOUT$", "w", stderr) == 0) {
_dup2(_fileno(stdout), 2);
}
std::ios::sync_with_stdio();
FlutterDesktopResyncOutputStreams();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void CreateAndAttachConsole() { | |
| if (::AllocConsole()) { | |
| FILE *unused; | |
| if (freopen_s(&unused, "CONOUT$", "w", stdout)) { | |
| _dup2(_fileno(stdout), 1); | |
| } | |
| if (freopen_s(&unused, "CONOUT$", "w", stderr)) { | |
| _dup2(_fileno(stdout), 2); | |
| } | |
| std::ios::sync_with_stdio(); | |
| FlutterDesktopResyncOutputStreams(); | |
| } | |
| } | |
| void CreateAndAttachConsole() { | |
| if (::AllocConsole()) { | |
| FILE *unused; | |
| if (freopen_s(&unused, "CONOUT$", "w", stdout) == 0) { | |
| _dup2(_fileno(stdout), 1); | |
| } | |
| if (freopen_s(&unused, "CONOUT$", "w", stderr) == 0) { | |
| _dup2(_fileno(stdout), 2); | |
| } | |
| std::ios::sync_with_stdio(); | |
| FlutterDesktopResyncOutputStreams(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@windows/runner/utils.cpp` around lines 10 - 22, The error check for freopen_s
is inverted in CreateAndAttachConsole: freopen_s returns 0 on success, but the
code calls _dup2 only when freopen_s returns non-zero; change the condition so
that _dup2(_fileno(stdout), 1) runs when freopen_s(&unused, "CONOUT$", "w",
stdout) returns 0, and likewise _dup2(_fileno(stdout), 2) runs when
freopen_s(&unused, "CONOUT$", "w", stderr) returns 0; keep the rest of the
function (AllocConsole, std::ios::sync_with_stdio(),
FlutterDesktopResyncOutputStreams()) unchanged.
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
1 participant
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.
🐛 Fix: Build crash due to missing key.properties
This PR fixes a crash that occurs when
key.propertiesis not present.✅ Changes:
as String) from keystorePropertieskey.properties.examplefor contributor setup guidance.gitignoreto exclude sensitive files🚀 Result:
Fixes #801
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Please include screenshots below if applicable.
Checklist:
Maintainer Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests