Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling for Studio updating process #928

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

nightnei
Copy link
Contributor

Related issues

Proposed Changes

I tried to reproduce the issue, I messed up file system and I managed to reproduce the error only from inside of DMG file.
But necessary to mention:

  1. I have M1 (Intel in the issue). Also, ofc, bundles are different.
  2. I have macOS Sequoia 15.2 (Sierra and latter versions disallowed to update from non-Application folders)
  3. I am working with Studio 1.3.3 (1.0.3 in the issue, maybe it was related to old Electron or something like that)
    Necessary to mention, that the error is thrown from Squirrel.Mac, so such issue is not specific to Studio and can be reprodicable for many different apps, so we can do not more then improving error handling.

So after different testings, I can confirm that update process work totally correctly in Studio 1.3.3 and almost latest macOS Sequoia 15.2.
What we can do (my proposed changes) - show more specific errors when it's running from inside DMG file. Also show errors to move the app to Applications folder in case if somebody accidentally encounter such error (I haven't managed to see this error) and as a last resort - show default message to check folder permissions and additionally in this case we will send Sentry error, to have move info about it, if somebody one day catch it somehow.

If this PR is merged - we can close the issue and ask the issue creator to try to reproduce it with Studio 1.3.4. So that we can catch this error in Sentry. But my assumption is that something is wrong specifically with installing Studio for that user, since during research of the error, I saw cases in some people, when just reinstalling an app fixed the issue. So I think that installing 1.3.4, after removing their version will fix it.

Testing Instructions

Apply the next diff:

diff --git a/src/updates.ts b/src/updates.ts
index 42f20a1..6a64de6 100644
--- a/src/updates.ts
+++ b/src/updates.ts
@@ -35,6 +35,7 @@ export function setupUpdates() {
        autoUpdater.setFeedURL( { url: url.toString() } );
 
        autoUpdater.on( 'checking-for-update', () => {
+               handleReadOnlyVolumeError( new Error( 'Checking for update' ) );
                updaterState = 'checking-for-update';
        } );
  • run npm run make

Test running Studio from inside of DMG file

  • Open /out/make
  • Click on Studio-1.3.3-arm64.dmg
  • Open Studio from dmg, instead of dragging the app to Apllications folder
  • Assert that you see the next error (ignore the fact about seeing 2 errors, it's expected from our diff above)
    Screenshot 2025-02-12 at 16 17 32

Test running Studio from Applications folder (It's literally unreal error, but it's exactly what encountered in this issue. My assumption that something is aliased/linked/etc, so with this error we will be sure, which path Studio sees)

  • Open /out/make
  • Click on Studio-1.3.3-arm64.dmg
  • Drag to Applications folder
  • Run it from there
  • Assert that you see the error from diff above
    Screenshot 2025-02-12 at 16 22 03

Test running Studio outside of Applications folder

  • Following previous test case, let's move Studio from Applications to Downloads (macOS Sequoia 15.2 allows to do it, unlike Sierra, so here will be used applied diff)
  • Run it from Downloads
  • Assert that you see the next error
    Screenshot 2025-02-12 at 16 26 17

@nightnei nightnei requested a review from a team February 12, 2025 16:48
@wojtekn
Copy link
Contributor

wojtekn commented Feb 13, 2025

@nightnei I'm unsure if providing the different errors adds value here. Wouldn't displaying the current dialog be enough so users who launch the app from DMG or launch it from DMG and trigger an update see this dialog? Doesn't the current dialog catch those cases? In my tests, I'm getting this dialog when I try to launch Studio from DMG:

Screenshot 2025-02-13 at 11 10 33

@nightnei
Copy link
Contributor Author

@wojtekn
While reading stackoverflow I noticed that some people by mistake, pin DMG version to dashboard and then every time they open it from dashboard. Also, there were cases when something was messed up by OS and it was thinking that it's opening from read-only volume and reinstalling an app fixed the issue.
So with changes in this PR I want:

  1. Show more specific errors, so that it can help our users to understand which instanceof Studio is running (in this PR, errors contain path to the instance).
  2. We don't log errors to Sentry. But in this specific issue, we had to log it, according to user's description, so I added logging errors if it happens from instances of Studio which are running not from DMG or Downloads folder.
  3. Also, with proposed changes, it will be easier for us to investigate future cases, since we won't need to clarify - whether they run it from DMG or maybe Downloads folder (on earlier OS) or maybe some other place. Such granularity will definitely help us and users. And this PR doesn't add some difficult logic, so IMO it worth it.

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.

Error when opening the app
2 participants