hyperspacedev / hyperspace

The new beautiful, fluffy client for Mastodon in React + TypeScript
https://hyperspace.marquiskurt.net
Other
182 stars 26 forks source link

[Bug] config.json not loading correctly in Linux desktop app #200

Closed ariasuni closed 4 years ago

ariasuni commented 4 years ago

Describe the bug

To Reproduce Steps to reproduce the behavior:

  1. Add an account on a Mastodon instance
  2. Go to About page

Expected behavior Fields aren’t shown if the information can’t be retrieved for Mastodon, or set clearly as not told by instance if the information can be given but isn’t available.

Screenshots Screenshot_20200417_203559

App Information (please complete the following information):

Additional context Instance on the screenshot is Eldritch Café.

alicerunsonfedora commented 4 years ago

I am not sure if the Debian conversion for Arch Linux users does anything funky in particular, but it appears that perhaps the conversion process removed config.json, the file responsible for providing the configuration settings inside of the app. These errors are actually the default values when this file is missing, so I'm not sure if our desktop build somehow deleted this file or if the conversion from the officially-supported Debian package to whatever is used on Arch Linux removed this file or not.

ariasuni commented 4 years ago

There is no config.json in the Debian package you provide. The «conversion» I’m talking about is just taking all the files from the Debian archive and repackaging them so that Arch Linux can install them.

alicerunsonfedora commented 4 years ago

The Debian package won't contain it directly, but it should be inside of the app.asar archive. Upon further inspection, the config.json file is present in the app.asar archive when downloading and extracting the Debian archive.

I'm pondering if this is related to #195 in any way given that this might be a resource loading issue. Is there anything present in the console when opening developer tools (Shift-Ctrl-I)?

alicerunsonfedora commented 4 years ago

We've just released v1.1.1, which fixes #195. Does this issue still appear with this new version?

ariasuni commented 4 years ago

When opening the devtools: Screenshot_20200417_231755

And when going to the About page, I see that in the console: GET https://eldritch.cafe/api/v1/accounts/774314 404

alicerunsonfedora commented 4 years ago

Hmm, so nothing related to the config.json file. Is this still present with v1.1.1?

Edit: And yes, that dialog is expected behavior, unfortunately, though it has no relevance to the issue in question

ariasuni commented 4 years ago

Exactly the same behavior.

If I had to guess with what I saw in the code and the error I got, one request fails so the rest of the code doesn’t execute and some fields are left blank/with default value.

alicerunsonfedora commented 4 years ago

This is rather unusual as I am unable to reproduce this on both macOS and the web browser, so I am unsure with what's going on here. If there is a request failing, it's probably the config issue which is most likely it not seeing config.json for some obscure reason. @travisk-codes , could you investigate?

alicerunsonfedora commented 4 years ago

After opening the about page and reading the previous issue, I suddenly see why this issue is present. The config.json file is being read; however, in our current implementation, the values get set if and only if the account in the config.admin.account field could be loaded by the API on the users' given instance. This is incorrect behavior as this one facet to the config should not affect the entire thing.

I've written a patch that puts most of the config loads into a finally statement in the promise instead of all inside the then, so this should mitigate the issue.

ariasuni commented 4 years ago

I’m looking at the code and it seems completely wrong to me. Hyperspace tries to load an account based on an hardcoded ID:

https://github.com/hyperspacedev/hyperspace/blob/a86b97c46c1e37e4d5dd50e2b00d16463f5fe13f/public/config.json#L18-L21

https://github.com/hyperspacedev/hyperspace/blob/a86b97c46c1e37e4d5dd50e2b00d16463f5fe13f/src/pages/About.tsx#L82

but the ID of an account depends of the instance. That means that for other instances, it will either fail to find an account with such ID, or find another account.

Instead, you can use /api/v2/search?q=@hyperspacedev@mastodon.social&type=accounts&limit=1 (/api/v2/search documentation) (I’m no Mastodon expert so it might not be the most efficient way to do this).

alicerunsonfedora commented 4 years ago

As of right now, the ID being dependent on the currently signed-in instance is more or less intended behavior at the moment since the ID is a hardcoded value that is relative to owner of the server that Hyperspace is running from, so it's not "wrong". However, we could change this in a future update, but it might break some compatibility with older config.json configurations and may be more expensive to do a search to extract the ID from it and use that instead.

Regardless, the value set in config.admin.account being relative to the signed-in server is not necessarily the exact cause of this issue (as initially filed) and should be made into a new bug report/feature request. The initial issue filed here should be fixed with #203 as we have moved the state updates in a finally statement so that it's not dependent on the account being fetched.

alicerunsonfedora commented 4 years ago

I've opened up a new request on #210 to update the admin account field to point to the proper ID. Regarding the about page not displaying all information, this should be fixed with #203 and will be shipped with v1.1.2.