Skip to content

Conversation

@seer-by-sentry
Copy link
Contributor

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Verified that file dialogs open correctly in various scenarios.
  2. Checked the application logs for any error messages related to COM exceptions or shell item creation failures.
  3. Confirmed that the application does not crash when file dialog operations fail.
  4. Tested opening and saving files in different locations to ensure the fixes do not introduce regressions.

Fixes FILES-APP-2PH. The issue was that: Windows COM subsystem failed to instantiate a file dialog object, indicating system-level corruption or critical resource failure.

  • Adds try-catch blocks to handle COMExceptions and other exceptions that may occur during file dialog operations.
  • Logs specific error messages with HRESULT values when COM object creation or dialog showing fails.
  • Gracefully handles shell item creation failures by logging a warning and continuing without setting the default folder.
  • Prevents crashes by returning false when exceptions occur, indicating that the dialog operation failed.

This fix was generated by Seer in Sentry, triggered by Yair. 👁️ Run ID: 831432

Not quite right? Click here to continue debugging with Seer.

@yaira2 yaira2 marked this pull request as ready for review August 15, 2025 15:15
@yaira2 yaira2 changed the title Reliability: Handle COM and other exceptions in file dialog services Code Quality: Handle COM and other exceptions in file dialog services Aug 15, 2025
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Aug 15, 2025
@yaira2 yaira2 requested a review from 0x5bfa August 15, 2025 15:19
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Since we don't have a good error handling (that is, a error dialog), even if we do this, the dialog just doesn't appear and user would think it's broken from the start. We'd just better let it crash.

And ALL of them never should go wrong; exiting gracefully makes it harder to catch up.

@yaira2
Copy link
Member

yaira2 commented Aug 17, 2025

Since we don't have a good error handling (that is, a error dialog), even if we do this, the dialog just doesn't appear and user would think it's broken from the start. We'd just better let it crash.

And ALL of them never should go wrong; exiting gracefully makes it harder to catch up.

An error message can be added, but in any event, I would prefer for it to look like nothing is happening than for the app to crash.

@0x5bfa
Copy link
Member

0x5bfa commented Aug 18, 2025

That case, sure.

@yaira2
Copy link
Member

yaira2 commented Aug 18, 2025

I'm curious why it's failing in the first place, can they be missing a service?

@0x5bfa
Copy link
Member

0x5bfa commented Aug 18, 2025

Based on the exception type which is COMException, one of the com methods are failing. Note that it should NEVER happen. Can think of the APIs not found *somehow* or the general memory failures happening.

@yaira2
Copy link
Member

yaira2 commented Aug 18, 2025

We have close to a dozen reports for this in Sentry.

@0x5bfa
Copy link
Member

0x5bfa commented Aug 18, 2025

This adds detailed error code to the log and I think log is also reported to Sentry in the same user's page, right? I don't have access to there but you should be able to knnow what the issue is from that.

@yaira2
Copy link
Member

yaira2 commented Aug 18, 2025

I don't see anything helpful in the stacktrace.

@0x5bfa
Copy link
Member

0x5bfa commented Aug 18, 2025

I mean there should be going to have the HRESULT code now because this adds log and the logging also reports to Sentry, right? After a release published after this PR, we should be knowing the error code, I think.

@yaira2
Copy link
Member

yaira2 commented Aug 18, 2025

Sounds like a plan 👍

@yaira2 yaira2 merged commit b7dd8cb into main Aug 18, 2025
9 checks passed
@yaira2 yaira2 deleted the seer/file-dialog-exception-handling branch August 18, 2025 21:24
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Pull requests that are approved and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants