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

Starting agent in background doesn't work with esbuilt bundle #34

Closed
CMCDragonkai opened this issue Oct 18, 2023 · 3 comments · Fixed by #39
Closed

Starting agent in background doesn't work with esbuilt bundle #34

CMCDragonkai opened this issue Oct 18, 2023 · 3 comments · Fixed by #39
Assignees
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Oct 18, 2023

Specification

The reason for this is quite simple. To run the agent in the background it relies on forking polykey-agent.js.

It looks like this:

        const agentProcess = childProcess.fork(
          path.join(__dirname, '../polykey-agent'),
          [],
          {
            cwd: process.cwd(),
            env: process.env,
            detached: true,
            serialization: 'advanced',
            stdio,
          },
        );

That's because the file CommandStart exists in src/agent directory. So it's trying to refer the file relative to the location of the script file.

However this no longer makes sense if the code is to be bundled together. Because then the location of the source script doesn't matter. What matters is the location of the bundled file.

This is a similar problem to the location of node native addons MatrixAI/js-quic#71. And it's also similar to the problem of loading polykeyWorker.js which is now re-exported as an additional entrypoint under dist/.

There 2 ways we could solve this.

  1. Combine polykey.ts and polykey-agent.ts together and use conditional logic to separate the different behaviour.
  2. Maintain the location of the primary/main script and pass that down to CommandStart, so it always uses the path relative to the main script. This allows us to assume that polykey-agent will always relative to the polykey script (right next to it). And then this could work.

Solution 2 would be the quickest, however I don't know if this will work with a pkg bundled executable. Does the VFS it creates also intercepts forking calls to the local filesystem? It seems like it does... because, that's what happened with the workers.

  return await WorkerManager.createWorkerManager<PolykeyWorkerModule>({
    workerFactory: () => spawn(new Worker('./polykeyWorker')),
    cores,
    logger,
  });

If that's always relative to the script executing, then that's why we needed dist/polykeyWorker.js, and thus setting it to the scripts property in the pkg configuration.

Additional context

@CMCDragonkai CMCDragonkai added the development Standard development label Oct 18, 2023
@CMCDragonkai CMCDragonkai self-assigned this Oct 18, 2023
@CMCDragonkai
Copy link
Member Author

Going to try solution 2 first to see if pkg can handle it. Apparently we use: const mainDir = path.dirname(process.execPath);.

But if it not, then the the scripts have to be combined.

We would need a way to tell the script that we want to run it in agent mode. 2 possibilities: env variable or --agent-mode flag. We just need to avoid conflict, then --agent-mode would look better. Then setting the process title wouldn't really be necessary. It should be an undocumented parameter though.

@CMCDragonkai
Copy link
Member Author

I think long term, the threads problem is solved by a proper embedding solution. While I think it would be better to just do conditional loading. We can also swap to dynamic imports between the 2 branches.

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Oct 18, 2023

Actually the process.execPath doesn't work. We would need to just know which script was the main script. There is a require.main.filename that works, but not in ESM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity
1 participant