Skip to content

Conversation

@isc-bsaviano
Copy link
Contributor

@gjsjohnmurray
Copy link
Contributor

Using the dev VSIX from this PR alongside the one from my Server Manager PR is doing a much better job of closing down web sessions when exiting, as well as when closing or reloading workspaces (server-side in my use cases). The one lingering /api/atelier session I am seeing may belong to Language Server, and I'm hoping your upcoming change there will tidy this one up.

I think there's perhaps an edge case which your current deactivate strategy doesn't handle.

  1. Create a new workspace to access a namespace on server A.
  2. Add a second folder to it which accesses a namespace on server B.
  3. Remove that folder from the workspace.
  4. Close the workspace.

I think a web session onto B will remain.

Also, when looking at this issue I tried to refresh myself about how this extension has historically stored session cookies in its extensionContext.globalState under "API:host:port" keys, though doing this via the Cache class from the vscode-cache package.

The new code for ending sessions during deactivate almost certainly has consequences for that mechanism, which I assume was intended to avoid re-prompting for credentials, and to share license resources.

However the "API:host:port" key doesn't handle the increasingly common scenarios where host:port serves multiple IRIS instances and pathPrefix is involved.

I don't think these globalState entries ever get deleted. Nor is it possible to do that during deactivate owing to how most of the extension API servicing framework has already been terminated by then.

IMO none of the above needs delay this PR, but I wanted to record the info before I forget it.

@isc-bsaviano
Copy link
Contributor Author

I think there's perhaps an edge case which your current deactivate strategy doesn't handle.

  1. Create a new workspace to access a namespace on server A.
  2. Add a second folder to it which accesses a namespace on server B.
  3. Remove that folder from the workspace.
  4. Close the workspace.

I think a web session onto B will remain.

Yeah, this is an edge case. IMO this logout behavior is a "nice to have" so I'm not worried about it being suboptimal.

Also, when looking at this issue I tried to refresh myself about how this extension has historically stored session cookies in its extensionContext.globalState under "API:host:port" keys, though doing this via the Cache class from the vscode-cache package.

The new code for ending sessions during deactivate almost certainly has consequences for that mechanism, which I assume was intended to avoid re-prompting for credentials, and to share license resources.

However the "API:host:port" key doesn't handle the increasingly common scenarios where host:port serves multiple IRIS instances and pathPrefix is involved.

I don't think these globalState entries ever get deleted. Nor is it possible to do that during deactivate owing to how most of the extension API servicing framework has already been terminated by then.

This code is very old, so it's not surprising that it doesn't take pathPrefix into account. It would be nice to do a review and cleanup of this in a separate PR.

@isc-bsaviano isc-bsaviano marked this pull request as ready for review April 29, 2025 20:23
@isc-bsaviano isc-bsaviano requested a review from isc-rsingh April 29, 2025 20:23
@isc-bsaviano isc-bsaviano merged commit 9d292ea into intersystems-community:master Apr 30, 2025
5 checks passed
@isc-bsaviano isc-bsaviano deleted the logout branch April 30, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants