Incorrect use of DISP_E_EXCEPTION in CPluginUserSettings::Invoke()

[eric@adblockplus.org]

Environment

Source code defect.

Observed Defect

Two of the calls to the user settings object are "by index". At present, we are returning DISP_E_EXCEPTION for these errors but we are not filling in the exception structure as specified here:

​https://msdn.microsoft.com/en-us/library/windows/desktop/ms221479(v=vs.85).aspx

How to Fix

One option is to fill in the exception structure.

Easier, however, would be to simply return DISP_E_BADINDEX. This is not documented on the above page, but it is supported elsewhere by Microsoft. After all, it's defined in the same header file as DISP_E_EXCEPTION is, just a few lines away. But also see, for example, the following SO question, which shows that (1) Excel returns this value and (2) the .NET runtime has support for interpreting it.

​https://stackoverflow.com/questions/16851642/c-sharp-excel-interop-exception-from-hresult-disp-e-badindex

[sergz]

Replying to eric@…:

One option is to fill in the exception structure.

I would proceed with this option. To make it easier we can create a helper method to fill EXCEPINFO and return DISP_E_EXCEPTION.

Easier, however, would be to simply return DISP_E_BADINDEX. This is not documented on the above page, but it is supported elsewhere by Microsoft. After all, it's defined in the same header file as DISP_E_EXCEPTION is, just a few lines away. But also see, for example, the following SO question, which shows that (1) Excel returns this value and (2) the .NET runtime has support for interpreting it.
​​https://stackoverflow.com/questions/16851642/c-sharp-excel-interop-exception-from-hresult-disp-e-badindex

I would suppose that in the link above IDispatch::Invoke is not called because of early binding, and DISP_E_EXCEPTION is return by DISPID_DEFAULT method and the means of interoperability ensures to raise an exception converted either directly from HRESULT or from COM exception.

Of course, on one hand we may return any error HRESULT code but on the other hand I would personally use only return codes mentioned on the ​documentation page. Most probably it does work in our case but it looks dirty in context of COM automation.

[eric@adblockplus.org]

I set up a live test on this issue; see ​https://codereview.adblockplus.org/29331713/

The results of the test are clear. When an IDispatch method returns DISP_E_BADINDEX, the JS statement throws an exception, it's prototype has 'name' property "RangeError", and the exception message is "Subscript out of range".

Thus, regardless of that fact that the error code is DISP_E_BADINDEX is not documented as a standard return from 'IDispatch', it is in fact fully supported within the JavaScript engine of IE.