hyperplay-desktop-client icon indicating copy to clipboard operation
hyperplay-desktop-client copied to clipboard

[FIX] Track Install Failure only if the installation Failed completely

Open flavioislima opened this issue 1 year ago • 5 comments

Right now if a game install successfully but it fails to remove temporary files, for instance, the client sends a Install Failed event. We should only track this if the game failed to install completely, this means that the install command returned status === error.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • [ ] Tested the feature and it's working on a current and clean install.
  • [ ] Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • [ ] Created / Updated Tests (If necessary)
  • [ ] Created / Updated documentation (If necessary)

flavioislima avatar Sep 13 '24 15:09 flavioislima

We won't be tracking several error states with the download manager with this change.

For instance, if the user starts a download, the platformInfo is not found, and the download stops with error, we won't be tracking that error if we remove this line.

Also, we also have a try catch block inside hyperplay.gameManager.install with this catch block

  } catch (error) {
    process.noAsar = false

    logInfo(
      `Error while downloading and extracting game: ${error}`,
      LogPrefix.HyperPlay
    )
    if (!`${error}`.includes('Download stopped or paused')) {
      callAbortController(appName)
    }

    return {
      status: 'error',
      error: `${error}`
    }
  }

So if hyperplay install throws during the process, we will not be tracking that error in analytics either.

So I don't think we should make this change.

We should consider removing this return { status: 'error' ... code as well and just rely on throw, try, catch. it is overcomplicated

BrettCleary avatar Sep 13 '24 16:09 BrettCleary

For the error codes we want to ignore what if we added error codes to the error we throw and we filter based on the error code whether to report the error to analytics @flavioislima

const error = new Error("message")
error.code = "YOUR_STATUS_CODE"
throw error;

Since I think the main issue is with canceling a download, it still throws and tracks.

BrettCleary avatar Sep 13 '24 16:09 BrettCleary

For the error codes we want to ignore what if we added error codes to the error we throw and we filter based on the error code whether to report the error to analytics @flavioislima

const error = new Error("message")
error.code = "YOUR_STATUS_CODE"
throw error;

Since I think the main issue is with canceling a download, it still throws and tracks.

Cancel actually sends the Abort status so it's not tracked as failed.

The majority of errors are Ebusy and some Eperm and a few others comes as undefined.

I will improve this logic then and if needed we can filter more errors in the future 👍

flavioislima avatar Sep 13 '24 16:09 flavioislima

@BrettCleary

We should consider removing this return { status: 'error' ... code as well and just rely on throw, try, catch. it is overcomplicated

The thing is that then we will be more limited. Because then an abort will also throw and error, but an abort can be a pause state and then we will have wrong metrics.

I honestly think we should track error in Sentry and here we only track if the Game Installed Successfully or not.

The way it is now it is adding a lot of noise and not giving us the right numbers on that.

For instance, if the user starts a download, the platformInfo is not found, and the download stops with error, we won't be tracking that error if we remove this line.

I don't think this is possible to do because we need the installInfo when the client opens the Install Dialog. Without it the Install button is disabled. So this is the type of error we should move to Sentry (cannot get install info and cannot get game info) , then we can have a better overview of gamings that are having it.

flavioislima avatar Sep 16 '24 10:09 flavioislima

@BrettCleary

About your first message, it is true that right now we are sending an error event when pausing or cancelling a HyperPlay download. I believe that those are the ones that are coming as error: undefined on the tracking system.

I changed now on my latest commit for it to return status: abort instead of an error. That should deal with it.

And for the events that were removed (EBUSY, EPERM, etc), I now changed them to be sent to Sentry with proper information for us to debug like the AppID, platform, etc.

flavioislima avatar Sep 16 '24 10:09 flavioislima