[V3] Windows: fix(application): handle error and type assertion in save file dialog #4284
Conversation
WalkthroughThe changes address error handling in the Windows SaveFileDialog implementation. They update error signaling when a dialog is cancelled, ensuring a specific error is returned instead of a generic one. The dialog result handling is made safer to prevent panics on type assertion failures. Documentation is updated to reflect the bug fix. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant SaveFileDialog
participant OS
User->>App: Initiate SaveFileDialog
App->>SaveFileDialog: show()
SaveFileDialog->>OS: Display dialog
OS-->>SaveFileDialog: User selects file or cancels
alt User selects file
SaveFileDialog->>App: Return file path (string)
else User cancels/closes
SaveFileDialog->>App: Return ErrorCancelled
end
App->>User: Process result or report error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
Documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/pages/changelog.mdx (1)
19-19: Good documentation of the fix in the changelog.The changelog entry clearly describes the issue that was fixed and references the PR number. This helps users and developers understand what was addressed in this release.
Note: "Windows" should be capitalized as it refers to the operating system.
- Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) + Fixed panic when closing or cancelling a `SaveFileDialog` on Windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284)🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling aSaveFileDialogon windows. Fixed in [PR](https://github.com/wails...(A_WINDOWS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
v3/internal/go-common-file-dialog/cfd/vtblCommonFunc.go(2 hunks)v3/pkg/application/dialogs_windows.go(1 hunks)website/src/pages/changelog.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/src/pages/changelog.mdx
[grammar] ~19-~19: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling a SaveFileDialog on windows. Fixed in [PR](https://github.com/wails...
(A_WINDOWS)
🔇 Additional comments (3)
v3/internal/go-common-file-dialog/cfd/vtblCommonFunc.go (1)
171-173: Improved error handling with specific error type.This change enhances error handling by returning a specific
ErrorCancellederror rather than a generic error message. This is a better practice as it allows calling code to check for specific error types rather than parsing error strings.v3/pkg/application/dialogs_windows.go (2)
199-202: Fixed potential resource leak by properly handling errors.This is a critical fix that properly handles errors from
showCfdDialogby closing the channel and returning early. Previously, an error could have resulted in a resource leak or incorrect behavior because the channel wouldn't be properly closed.
205-208: Added type safety to prevent panic on invalid type assertion.This change adds proper type assertion with a check, preventing a potential panic if the result is not of the expected type. This is good defensive programming that makes the code more robust against unexpected scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/content/docs/changelog.mdx (1)
131-131: Capitalize “Windows” and align entry style.The new fix entry currently reads “on windows.” Windows is a proper noun and should be capitalized. Also for consistency with other OS‐specific entries, consider adding the ⊞ badge and using the
[#4284]PR reference style:- - Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) by @hkhere + - ⊞ Fixed panic when closing or cancelling a `SaveFileDialog` on Windows. Fixed in [#4284](https://github.com/wailsapp/wails/pull/4284) by @hkhere🧰 Tools
🪛 LanguageTool
[grammar] ~131-~131: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling aSaveFileDialogon windows. Fixed in [PR](https://github.com/wails...(A_WINDOWS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/content/docs/changelog.mdx(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[grammar] ~131-~131: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling a SaveFileDialog on windows. Fixed in [PR](https://github.com/wails...
(A_WINDOWS)
|
dosubot
bot
added
the
size:S
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/content/docs/changelog.mdx (1)
28-30: Capitalize “Windows” and add platform icon for consistencyThe changelog convention (see legend above) prefixes Windows-specific fixes with “⊞” and capitalizes the OS name. Keeping this consistent improves scan-ability.
- - Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) by @hkhere + - ⊞ Fixed panic when closing or cancelling a `SaveFileDialog` on Windows. See [PR](https://github.com/wailsapp/wails/pull/4284) by @hkhere
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/content/docs/changelog.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: fbbdev
PR: wailsapp/wails#4100
File: v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.ts:222-228
Timestamp: 2025-02-25T17:13:05.652Z
Learning: The user has chosen to maintain the function name `Error` in the dialogs module despite it shadowing the global Error constructor. This is a deliberate choice due to documentation generator compatibility concerns, and the risk is mitigated by using `window.Error` when referencing the global constructor and by the strictly type-checked modular nature of the code.
docs/src/content/docs/changelog.mdx (5)
Learnt from: popaprozac
PR: wailsapp/wails#4256
File: v2/internal/frontend/desktop/linux/notifications.go:27-28
Timestamp: 2025-04-29T23:54:07.488Z
Learning: In Wails v2, unlike v3-alpha which has a `ServiceShutdown` method for services, there is no standardized teardown pattern for frontend implementations. When implementing features that require cleanup (like goroutines or resources), add explicit cleanup methods (e.g., `CleanupNotifications()`) that handle resource release, context cancellation, and connection closure.
Learnt from: popaprozac
PR: wailsapp/wails#4256
File: v2/internal/frontend/desktop/linux/notifications.go:27-28
Timestamp: 2025-04-29T23:54:07.488Z
Learning: In Wails v2, unlike v3-alpha which has a `ServiceShutdown` method for services, there is no standardized teardown pattern for frontend implementations. When implementing features that require cleanup (like goroutines or resources), add explicit cleanup methods (e.g., `CleanupNotifications()`) that handle resource release, context cancellation, and connection closure.
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_darwin.go:39-46
Timestamp: 2025-03-23T00:41:39.612Z
Learning: For the macOS notifications implementation in Wails, an early panic is used when the bundle identifier check fails rather than returning an error, because the Objective-C code would crash later anyway. The panic provides clear instructions to developers about bundling and signing requirements.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[grammar] ~29-~29: There might be a mistake here.
Context: ...github.aichem.org//pull/4284) by @hkhere ## v3.0.0-alpha.10 - 2025-07-06 ### Break...
(QB_NEW_EN_OTHER)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v3/UNRELEASED_CHANGELOG.md (1)
24-27: Style: align wording with project changelog guidelinesUse present-tense verb (“Fix”), capitalize “Windows”, and consolidate the sentence to avoid repetition.
- - Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) by @hkhere + - Fix panic when closing or cancelling a `SaveFileDialog` on Windows (#4279, #4284) by @hkhere
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/UNRELEASED_CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
v3/UNRELEASED_CHANGELOG.md
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ncelling a SaveFileDialog on windows. Fixed in [PR](https://github.com/wailsapp/wai...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
Description
This fixes a failure when closing or cancelling a
SaveFileDialogon windows.Ensure the save file dialog properly handles errors and safely asserts the result type to avoid potential runtime panics. Additionally, improve error handling in the file dialog result retrieval to return a specific error when the operation is cancelled.
Fixes #4279
Type of change
Please select the option that is 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 using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Bug Fixes
Documentation