Fixed crash when receiving non valid JSON from server.

[Sveske-Juice]

Hi I fixed a bug where if you connect to a server which returns invalid server information, like responding with HTML instead of the server manifest data when requesting for server information in getServerInfo(baseurl, auto_connect) the program would silently crash and display "Connecting..." on the screen meanwhile the request actually failed because it tried to parse the JSON data which was invalid.

The issue originates in frontend/js/ajax.js where there is no check if the parsed data upon a 200 OK response actually was parsed successfully and no error occurred:

xhr.onreadystatechange = function () {
    if (xhr.readyState == 4) {
        if (xhr.status == 200)
            if (settings.success)
                settings.success(JSON.parse(xhr.responseText));
    }
// ...

You can see there is no check when trying to parse the response data, which could lead to a silent crash.

The issue occurs for example if you have set up Jellyfin with nginx to be in a subfolder: <HOST_NAME>/jellyfin. The issue will then happen if you try to connect to <HOST_NAME>/jellyfinAAAAA because the nginx server will return a 302 response redirecting the request to <HOST_NAME>/jellyfin/web/index.html and response with HTML data, where the parsing of the response will fail and display "Connecting..." forever.

The issue also occurs for example when connecting to the demo Jellyfin server at ´https://demo.jellyfin.org/stablebut without thestableat the end, sohttps://demo.jellyfin.org.getServerInfo(baseurl, auto_connectwill try to requesthttps://demo.jellyfin.org/System/Info/Publicbut get redirected tohttps://demo.jellyfin.org/stable/web/index.html` which results in a crash because it tries to parse HTML.

My fix is very simple, its just to check if there happens a error while trying to parse the data, and if so notify the user instead of crashing and display "Connecting..." forever. It also makes errors which are a of type string display in the error banner instead of resulting in an unknown error.

[Sveske-Juice]

I have now updated the code with your suggested changes. It now throws an unknown error instead of "true", and it displays the HTTP status code with the error message upon a failed request, which actually did not work before (it would just show unknown error)

[Sveske-Juice]

Formatting is now fixed.

[Sveske-Juice]

Oh... I think i made a mess...

I don't know if I actually squashed the commits correctly. What should I do in this situation?

[dmitrylyzo]

Oh... I think i made a mess...

I don't know if I actually squashed the commits correctly. What should I do in this situation?

• git checkout -B master 55676ca8c88e395560b066ebbb0323db8fc6e499 This returns you before merge commits. It can be done via git reset HEAD~3, but checkout is more straight forward.
• check the commits
• force-push

As for squashing/splitting, (if you want to practice):

• Always check commits before pushing.
• Create a backup branch: git branch bak (assuming you are on your master HEAD). If you messed up git checkout -B master bak (-B to override existed branch).
• Next you need to do an interactive rebase. git rebase -i 0e3c02b342c60be42faa90621bf8101348dbe352 (you need to use jellyfin-webos master HEAD, I used the exact commit).
• You should see rebasing script where you need to rearrange commits and mark fixups (squash). For example, you can use fixup on c86bc4f replace 'event.target.status' with 'xhr.status'. This will apply c86bc4f to the previous commit (687646d Display error message with HTTP status code instead of showing unkwon error). Next, you can move 1b9454e No need for event param when using xhr.status right after Display error message with HTTP status code instead of showing unkwon error and also use fixup.

The general idea is to make commits cleaner by moving buggy changes to where they first appear.

[Sveske-Juice]

Thank you for explanation, it makes much more sense now, I think I have done it correctly now... I see why it is convenient to keep a backup branch in case something goes wrong haha :D.

I have squashed the commits into two which is about the invalid JSON issue and to show the HTTP status code as part of the error message. Is this how it's supposed to be done, or should it be squashed into a single commit?

[dmitrylyzo]

I moved some changes to the first commit: https://github.com/dmitrylyzo/jellyfin-webos/tree/fix-json-crash By using edit when rebasing. There were few minor conflicts.

[Sveske-Juice]

Nice! Do you think something is missing or is it ready to be merged?

[dmitrylyzo]

Nice! Do you think something is missing or is it ready to be merged?

If you don't mind, I can force push the branch I posted to your master.

[Sveske-Juice]

If you don't mind, I can force push the branch I posted to your master.

Yeah that is fine