-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Invalid server config causes hang #2278
Comments
This is no longer happening as of latest master. It was fixed a few days ago: https://github.com/jellyfin/jellyfin-vue/blob/master/frontend/src/store/index.ts#L48 |
I tested this again on the live demo site at https://jf-vue.pages.dev/ running 19abb4d and I'm still seeing the same result, I don't think this is fixed. |
@pmdevita After a 2nd re-read, you're right (in fact, this bug is something that I'm aware of but needs some huge work to get solved properly), my bad. Given you were mentioning the WebSocket connection (which in Firefox, given it doesn't support the Network Information API, was the only source of truth for the connection to the server), I thought you were talking about #2269, which was fixed by #2271, making a Ping to the server as an additional safe guard. I'm reopening this. |
I'm still hesitant though about the real scope of the issue: I agree that if the server is unreachable you will be greeted by an endless loading screen, but I'm not sure we should handle other cases like manually modifying the localStorage. The consistency of the data stored in localStorage data should be guaranteed as long as it's the client who access it. We can't, by any means, expect that the client acts fine if it was tampered by the user. In fact, doing that might pose a huge security risk. |
I originally encountered this issue when I changed the domain name of my Jellyfin server, I just figured editing your local storage would be an easier strategy to reproduce the issue. Probably should have clarified that a bit more, sorry. When I did rename my Jellyfin server's domain, the Jellyfin app just brought me to the server config/login screen again so that's my rational for doing the same here. |
Yep this fixes the problem. I think there's a chance requests to the server may hang and cause the loading bar and cursor to stay on for a while, but those eventually do go away and don't block any functionality. This can probably be closed now. |
@pmdevita I confirm you this chance exists ^^. Bear in mind that the caching/reactivity layer I implemented in #2201 with the client/server state is right now a "best effort". Somewhat the client is already capable of working offline but there are some places/caveats which still expect to be online (images is the most basic example that cames to my mind). Those kind of edge cases I'm aware of and are minor annoyances that currently goes out of scope, but it's true that what you pointed in this issue is a major problem which could render the client completely unusable in many common situations (reinstalling the server, for example). Although we still have those minor caveats, I think we have this major concern solved and the other details will be solved along while the frontend matures or in other issues, so I'm going to close this if you don't mind 👍. |
Description of the bug
Jellyfin Vue stores configuration about the server you are connected to in a JSON string in Local Storage and on page load, the app tries to establish a websocket connection to it. If the connection fails, the app will retry forever to connect to the server.
Steps to reproduce
You can reproduce this by going to Local Storage in your browser's developer tools and editing the JSON string attached to the
auth
key. Settingauth.servers[0].PublicAddress
to any URL that does not have a Jellyfin instance behind it will cause the app to hang on refresh.Expected behavior
Retrying should probably be limited, and after a certain number of fails, the user should be sent to the menu to add a server again.
Logs
No response
Screenshots
No response
Platform
macOS
Browser
Firefox
Jellyfin server version
10.8.13
Additional context
No response
The text was updated successfully, but these errors were encountered: