Fatal error when API credentials are not valid

[jmaas-onoffice]

When I enter invalid API credentials (for example I mistype the token, or I switch token and secret), on every page load there is a fatal error that prevents the UI from fully showing. This breaks the entire website.

To get the website working again, you need to disable the plugin, e.g. by renaming its folder via FTP, and then go to /wp-admin/options.php and fix the credentials (search for onoffice-settings-apisecret). Or you can deinstall and reinstall the plugin to reset the settings.

The error message is:

[07-Nov-2022 09:29:29 UTC] PHP Fatal error:  Uncaught {
    "actionid": "urn:onoffice-de-ns:smart:2.5:smartml:action:get",
    "resourceid": "",
    "resourcetype": "estateCategories",
    "parameters": []
}

errorCode: 500

onOffice\WPlugin\API\APIEmptyResultException in /www/htdocs/w01bb2af/testinstanz-jmaa.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientExceptionFactory.php:50
Stack trace:
#0 /www/htdocs/w01bb2af/testinstanz-jmaa.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientActionGeneric.php(85): onOffice\WPlugin\API\APIClientExceptionFactory->createExceptionByAPIClientAction(Object(onOffice\WPlugin\API\APIClientActionGeneric))
#1 /www/htdocs/w01bb2af/testinstanz-jmaa.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientActionGeneric.php(136): onOffice\WPlugin\API\APIClientActionGeneric->generateException()
#2 /www/htdocs/w01bb2af/testinstanz-jmaa.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/Fieldnames.php(18 in /www/htdocs/w01bb2af/testinstanz-jmaa.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientExceptionFactory.php on line 50

[jmaas-onoffice]

@dai-eastgate @LongTrong-exe This is quite a critical bug. The cycle ends Wednesday anyway, so finish up the bets, but then it would be great if you could prioritize diagnosing this bug to figure out a fix.

[dai-eastgate]

@dai-eastgate @LongTrong-exe This is quite a critical bug. The cycle ends Wednesday anyway, so finish up the bets, but then it would be great if you could prioritize diagnosing this bug to figure out a fix.

We will check and fix it.

[jmaas-onoffice]

If there is not a trivial fix, we can figure out a solution together. ;)

[jmaas-onoffice]

This not only happens, when I switch the token and secret, but whenever the credentials are invalid.

[dai-eastgate]

I think if api errorCode 500 instead of showing error we can show message to user and update default(API token and API secret). What do you think about this?

[jmaas-onoffice]

Oh, so it seems that the API was returning a different error code than our code expected. And because our code only handled that one specific error code, the exception was unhandled and crashed everything.

I just tested it again and now I can't reproduce the problem. It seems the API now responds with the expected error code.

But this issue demonstrates that we cannot allow exceptions to go unhandled here. I think it would be nice, though, to distinguish between a problem with the credentials and another exception.

So could we add an additional error message, when it's not an invalid credential exception, but something unexpected? In that case we would not want to tell the user that something is wrong with their credentials, but instead just that there is a problem with the API. In any case, we cannot allow unhandled exceptions here.

[dai-eastgate]

@jmaas-onoffice I added an additional error message when it is not an invalid credential exception. In the example, I reproduced the error. You can watch the video and give me feedback.

https://user-images.githubusercontent.com/106214469/201021581-462d0a54-89e7-4ce1-b406-26c8f4d2f5c5.mp4

[jmaas-onoffice]

That looks good! The other pages are usable. 👍

So the most important requirement is that no exception should make all pages unusable. It's ok if the plugin's pages are unusable.

We could also improve the error message a bit:

""" The onOffice plugin has an unexpected problem when trying to reach the onOffice API.

Please check the onOffice server status to see if there are known problems. Otherwise, report the problem using the support form. """

When the fix is done, feel free to merge it into master and notify me. :)

[dai-eastgate]

That looks good! The other pages are usable. 👍

So the most important requirement is that no exception should make all pages unusable. It's ok if the plugin's pages are unusable.

We could also improve the error message a bit:

""" The onOffice plugin has an unexpected problem when trying to reach the onOffice API.

Please check the onOffice server status to see if there are known problems. Otherwise, report the problem using the support form. """

When the fix is done, feel free to merge it into master and notify me. :)

@jmaas-onoffice yes, I fixed it and merged it into master