CollectionInterface: add JSON error handling to avoid plugin crash

[pryrt]

• To address specifics of ozone10’s crash report ([BUG] Failed download causes crash #13), check to make sure that char-vector after download is >0 characters and has the right first character for JSON ([ or {, depending on download).
• Also add exception handling around the JSON parsing and data crunching, in case there are other problems that I have not anticipated

closes #13

[pryrt]

@ozone10, feel free to test out the artifact.

During my debug, when I forced download problems (404 errors, empty return vector), it properly caught and showed a MessageBox without crashing the plugin. And if I force JSON errors (mismatched { or [, for example), the exception is still caught and handled without crashing the plugin.

This should be sufficient for your problem, and future JSON-parsing issues.

[ozone10]

I would use only if (!vcThemeJSON.empty()) and remove current if block,.
I don't expect .toc.json file content to be wrong.
In

NppPlugin-CollectionInterface/src/Classes/CollectionInterfaceClass.cpp

Line 57 in 374be61

std::vector<char> CollectionInterface::downloadFileInMemory(const std::wstring& url)

there are already message boxes with error description, and here seems to be redundant, And it can be annoying when you have to dismiss 4 message boxes and in the end get empty listbox.

[pryrt]

I would use only if (!vcThemeJSON.empty())

This would not catch the problem of GitHub sending a 404 or server error, because the vector will actually be the string of the error code (like 404 Not Found)

[ozone10]

This would not catch the problem of GitHub not sending you any data: when there is a download error, the vector is not empty, it gives the HTTP error code (like 404 Not Found)

I see in that case you can add check for empty vector, to reduce message boxes.

In what situation does it give four message boxes? I can see that it might give 2 -- one for themes and one for UDL -- but I cannot see how it could possibly give 4.

I have a firewall configured to block applications the first time they try to connect to net, so I can to decide whether to allow them.

[pryrt]

And it can be annoying when you have to dismiss 4 message boxes and in the end get empty listbox.

It took me a while to figure out how there could be 4 message boxes. But I'm assuming now that your reported "connection is blocked" was actually one of the "Could not connect to internet when trying to download" message boxes that the plugin launches from inside the downloadFileInMemory() function.

Now that I think I've figured that out, I can figure out a good way to non-annoyingly handle that condition -- while still preventing other possible problems, like a 404 or 500 error from the GH server, or receiving junk rather than JSON for unknown reasons. (Because there are multiple types of errors, there will still be the extra indentation -- but since it's my code, I will do the blocks the best way for me to understand for future maintenance)

[ozone10]

Another issue when clicking on "Download" button with empty listbox notepad++ will crash.

[ozone10]

Same as with vcThemeJSON but instead I would use

if (vcUdlJSON.empty())
{
    return;
}

to return early and reduce indentation level for next block for better readability.

[ozone10]

@pryrt
no more crash. 👍

[pryrt]

With the newest commit:

• When populating the lists from the download, if it gets an internet error (like the firewall block), the download function will still give the first message box, and will then immediately return FALSE to the main dialog box.
• If there is 404/500/etc when downloading Theme data, it will give a message box indicating there’s an error, and then try to move forward to the UDL (hoping that UDL will get valid data; since UDL is much more likely to be desired than Theme, I want to not stop a UDL download just because there’s a problem with the Theme info)
• If there is a 404/500/etc when downloading the UDL data: if Theme already gave the error message box, just return FALSE at this point; if Theme didn’t give the error message, then UDL will give error message and then return FALSE.
• If the list-populating exits with FALSE, then the main Dialog will close itself, because the user has already been informed of a problem.
• If somehow the file list is empty and the user clicks DOWNLOAD, it will give an error message, not crash

That means that it should do, at most, 1 message box saying something went wrong; it shouldn't show the download-dialog if there isn't good UDL data for populating the lists; and if one of the lists is still empty (like the Theme list), accidentally hitting DOWNLOAD should warn the user instead of crash.

I believe this addresses all your concerns.

[ozone10]

@pryrt
working great.