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

fix(daemon,cli): fix #2700 use endo/init to prepare async hooks #2708

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erights
Copy link
Contributor

@erights erights commented Feb 2, 2025

Closes: #2700
Refs: #2701

Description

Implements exactly @mhofman 's suggestion at #2700 (comment)

Alternative to #2701

Changes some occurrences of import 'ses'; to import '@endo/init'; so that the async hooks are properly prepared so that things work in an interactive debug session -- at least in vscode. See #2700

However, there are many other bare import 'ses'; occurrences in endo. How many for of these need to import '@endo/init'; instead in order to avoid this same problem? For now, I conservatively looked for other occurrences of the common marking comment "Establish a perimeter". I found two additional ones that this PR currently just annotates with a TODO. Reviewers, please advise.

If we take this approach, can we avoid needing to teach developers not to import 'ses'; so they avoid this problem? Or should we move the async-hook preparation somehow from @endo/init in ses, perhaps as a repair, so all existing import 'ses'; occurrences become correct?

Or should we instead proceed with #2701 after all?

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

Weird to need to teach developers to avoid import 'ses';. I hope we can avoid needing to teach this. Reviewers please advise.

Testing Considerations

The underlying issue #2700 occurs during an interactive debug session. If this is not fixed, it can impede the run-debug-loop, especially for running tests. In particular, ses-ava currently import 'ses'; rather than import '@endo/init';. This would perhaps be the highest next priority to change.

Compatibility Considerations

Perhaps not all occurrences of import 'ses'; can or should be switched to import '@endo/init'; because of the other things bundled into @endo/init.

Upgrade Considerations

I think none. Reviewers: please double check me on this.

@erights erights force-pushed the markm-2700-daemon-uses-endo-init branch from cb361c8 to 730ee1e Compare February 7, 2025 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In vscode debug session, node async_hooks promise keys appear too late
1 participant