Skip to content
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

Plugin API: Theia IDE doesn't support 'files` property of 'workspace/configuration' #5945

Open
vrubezhny opened this issue Aug 14, 2019 · 10 comments
Labels
bug bugs found in the application preferences issues related to preferences vscode issues related to VSCode compatibility

Comments

@vrubezhny
Copy link
Contributor

Description

PHP Intelephense extension fails to start due to lack of support of workspace configuration by Theia which is required in following call examples (https://github.com/bmewburn/vscode-intelephense/blob/master/src/middleware.ts#L278):

        let vscodeAssociations = workspace.getConfiguration('files').get('associations') || { };
...
        let vscodeExclude = workspace.getConfiguration('files', resourceUri || null).get('exclude') || { };

Reproduction Steps

Download PHP Intelephense extension v.1.1.4 (https://github.com/bmewburn/vscode-intelephense/releases) and save it to Theia 'plugins' folder, start Theia IDE browser example and try to edit/hover over/get content assist on a PHP file.
See errors listed above in the Output View.

OS and Theia version:
The issue is reproducible on Theia 'master' and 'che-7.0.0' branches

Diagnostics:
Errors from the Output view:

[Trace - 3:06:48 PM] Sending response 'workspace/configuration - (1)'. Processing request took 7ms
Result: [
    null,
    null
]
(node:31346) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'files' of null
    at Object.t.getDeepObjectValue (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:40:88018)
    at X (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:10077)
    at Object.<anonymous> (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:13127)
    at Generator.next (<anonymous>)
    at o (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:6813)
@vrubezhny
Copy link
Contributor Author

In VS Code before the configuration is stringified (to make it ready for sending it to PHP intelephense LS process) the extension tries to overlap some of the values with the result of mergeAssociations/mergeExclude and telemetry stuff. It's done in then() call on the array of two preference objects that came from the workspace and intelephense configs. So, because we cannot call then() on the initial configuration object (which is an array described above), this initial config gets resolved on Promis (Promis.resolve(init_config_array_object) is called at https://github.com/bmewburn/vscode-intelephense/blob/master/src/middleware.ts#L314).
Now it is possible to call then() on the resolved object and process every array item in this call,
But... In case of running the extension in VS Code the resulting object CAN be successfully stringified (by calling JSON.stringify(r)), while in case of running it in Theia the resulting object CANNOT be successfully stringified. So, when running in Theia, such an empty config is passed to the LS which in its turn immediately reports the following error:

[Trace - 3:06:48 PM] Sending response 'workspace/configuration - (1)'. Processing request took 7ms
Result: [
    null,
    null
]
(node:31346) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'files' of null
    at Object.t.getDeepObjectValue (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:40:88018)
    at X (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:10077)
    at Object.<anonymous> (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:13127)
    at Generator.next (<anonymous>)
    at o (/tmp/vscode-unpacked/vscode-intelephense-client-1.1.4.vsix/extension/node_modules/intelephense/lib/intelephense.js:53:6813)

The same goes for the newest version of the extension plugin, but in this case the LS reports that it cannot read telemetry property while the reason is the same - an empty config is transferred to the LS, so the issue #5944 looks like a duplicate of this one

@bmewburn
Copy link

bmewburn commented Oct 1, 2019

Hi, I stumbled across this. I'm open to changing to something like below in vscode-intelephense 1.3 if it helps.

let result = next(params, token);
if(!isThenable(result)) {
    return mergeResult(result);
} else {
    return result.then(r => mergeResult(r));
}

@danidoedel
Copy link

Hey @vrubezhny, any thoughts on this proposal? Do you think this would work?

@vrubezhny
Copy link
Contributor Author

@bmewburn Hello, sorry for the late answer...

I tried to follow your proposal, but it wasn't helping. Every time I start PHP editor for the first time in Che7 workspace the extension starts working (shows tooltips, content assist, etc.), but after I close and reopen the same file or open some other PHP file in editor it complains on empty configuration and refuses to work:

[Info  - 12:56:38 PM] Initialising intelephense 1.2.3
[Info  - 12:56:38 PM] Initialised in 18 ms
(node:19) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'telemetry' of null
    at Object.t.getDeepObjectValue (/tmp/vscode-unpacked/redhat.php.1.2.3.pszfvpceyo.vscode-intelephense-client-1.2.3.vsix/extension/node_modules/intelephense/lib/intelephense.js:59:58549)
    at Z (/tmp/vscode-unpacked/redhat.php.1.2.3.pszfvpceyo.vscode-intelephense-client-1.2.3.vsix/extension/node_modules/intelephense/lib/intelephense.js:72:30641)
    at j (/tmp/vscode-unpacked/redhat.php.1.2.3.pszfvpceyo.vscode-intelephense-client-1.2.3.vsix/extension/node_modules/intelephense/lib/intelephense.js:72:26502)
    at Object.<anonymous> (/tmp/vscode-unpacked/redhat.php.1.2.3.pszfvpceyo.vscode-intelephense-client-1.2.3.vsix/extension/node_modules/intelephense/lib/intelephense.js:72:35855)
    at Generator.next (<anonymous>)
    at o (/tmp/vscode-unpacked/redhat.php.1.2.3.pszfvpceyo.vscode-intelephense-client-1.2.3.vsix/extension/node_modules/intelephense/lib/intelephense.js:72:25467)
(node:19) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:19) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
[Error - 12:57:05 PM] Notification handler 'textDocument/didOpen' failed with message: Cannot read property 'files' of null

The extension plugin I had used to test was built with the following change: vrubezhny/vscode-intelephense@07c6155

@bmewburn Please review my change - maybe I did it wrong way or I missed something?
(Asking you to look at it because when I started it for the very first time (and there was a lot of console.info(...)-like lines to watch what's happening during the config merge) it was working fine and stopped working after I cleaned up the code ant rebuilt the plugin extension.)

@vrubezhny
Copy link
Contributor Author

PS: Now I see how to get it working for any number of PHP editors in Che7 : When I open a second (third, etc.) PHP file in editor PHP extension plugin refuses to work:

errorOnSecondFileOpen

So, to make it work again a user should leave the workspace page, for example, by opening the workspaces configuration page, and then back opening the same workspace - the editors will stay open and PHP extension will be fully functional in all the open editors (any attempt to close/reopen an editor or open any other PHP file in new editor will result again in non-functional PHP facilities on that editor)

@vrubezhny
Copy link
Contributor Author

@bmewburn Honestly, I don't understand what's going on there... The pluging extension starts working if I add console,info(...) calls into mergeResult()method like this:

    function mergeResult (r: any, params: ConfigurationParams) {
        console.info('mergeResult: r: ' + (r === null ? 'NULL' : r.toString()));
        console.info('mergeResult: oarams: ' + (params === null ? 'NULL' : params.toString()));
        if(Array.isArray(r)) {
            console.info('mergeResult: r is an Array');

            r.forEach((v, i) => {
                if(v && v.files && v.files.associations) {
                    v.files.associations = mergeAssociations(v.files.associations);
                }
                if(v && v.files && v.files.exclude) {
                    v.files.exclude = mergeExclude(v.files.exclude, params.items[i].scopeUri);
                }
                if(v && v.telemetry === null) {
                    v.telemetry.enabled = workspace.getConfiguration('telemetry').get('enableTelemetry');
                }
            });
        }
        console.info('mergeResult: result: ' + (r === null ? 'NULL' : r.toString()));

        return r;
    }

and rebuild the plugin.
That's it: the change proposed by you plus few console.info(...) calls fix the problem.

The intelephense log in this case stays almost the same but the following line disappears in this case:

[Error - 5:00:06 PM] Notification handler 'textDocument/didOpen' failed with message: Cannot read property 'files' of null

So, having these console.info(...) calls in place somehow fixes empty config in 'textDocument/didOpen' handler inside the server

@nickboldt
Copy link

nickboldt commented Dec 10, 2019

@bmewburn @vrubezhny Just found this issue as we're in the process of making sure that all 3rdparty-built vsix files can be built by RH productization so we can include our own bits in product (because product security, woo).

Has anyone tried using the latest intelliphense 1.3.3 with Che 7.5 or newer? Figure if we can skip over 1.0.x, 1.1.x, and 1.2x and go directly to 1.3, that would make it easier for productization, Che plugin reg management, and upstream project too!

cc: @mkuznyetsov ref eclipse-che/che#14677

@danidoedel
Copy link

Sadly still the same errors in Che 7.5.0 for me.

@bmewburn
Copy link

In 1.3 I refactored some of that middleware code which doesn't seem to have done anything. I'll take another look.

@nickboldt
Copy link

Thanks @bmewburn !

BTW I hope no one you knew was on the cruise ship that was affected by the White Island eruption earlier this week in NZ. (I visited that volcano last month. Crazy seeing the videos of the destruction.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application preferences issues related to preferences vscode issues related to VSCode compatibility
Projects
None yet
Development

No branches or pull requests

5 participants