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

Make timeout for shell.run configurable #1172

Open
gorootde opened this issue Nov 27, 2024 · 1 comment
Open

Make timeout for shell.run configurable #1172

gorootde opened this issue Nov 27, 2024 · 1 comment
Labels
AI implementation candidate Candidate issue to be implemented using AI tools enhancement New feature or request

Comments

@gorootde
Copy link
Contributor

I am using spacescript / a custom plug to call a CLI that will dump my calendar data as JSON. Such a call can take up to 30 seconds or even longer (depending on the amount of data returned).

Silverbullet aborts the shell.run sys call after 30 seconds, since this is the default timeout set here

I'd like to make this timeout configurable. Preferably per individual system call.

I am willing to contribute this, however I'd need some hint on how to add an additional parameter to the sys call.

@terr-steak
Copy link
Contributor

TLDR; Branch with code changes & script

Sorry I don't have a complete solution but, I spent some time looking into this and was able to pass in a custom timeout. Hopefully this is useful anyway :) ... I am not sure how much of this you may or may not have already figured out so I apologize for adding anything that you may already know, anyway what I did was the following and I was able to pass in a 50 second timeout...

It appears, like you pointed out that, the http_space_primatives.authenticatedFetch(...) function is what is causing the timeout, and it actually has a fetchTimeout parameter that defaults to the defaultFetchTimeout of 30 seconds, so if I understand correctly we want to be able to pass a timeout value to the shell.run syscall so that when it calls authenticatedFetch(...) here it passes a timeout.

Setup

  1. I created this script to test (${REPO_HOME}/scripts/trs-test.sh):
#!/bin/bash
echo "Starting test script"

# Default sleep duration
DEFAULT_SLEEP=40

# Check if an argument is provided
if [ -z "$1" ]; then
  echo "No argument provided. Using default sleep duration: $DEFAULT_SLEEP seconds."
  sleep $DEFAULT_SLEEP
else
  # Validate the input to ensure it's a positive number
  if [[ "$1" =~ ^[0-9]+$ ]]; then
    echo "Sleeping for $1 seconds."
    sleep $1
  else
    echo "Invalid input: '$1' is not a positive number. Exiting."
    exit 1
  fi
fi

echo "Done sleeping!"
  1. Then I created a corresponding space script to call it:
silverbullet.registerFunction({name: "trsTest"}, async (timeout) => {
  let args;
  if (timeout) {
    args = [timeout]
  } else args = [];
  
  // script is relative to the data-dir which is at the same level as `${REPO_HOME}` ie. `../silverbullet`
  result = await shell.run("../silverbullet/scripts/trs-test.sh",args, 50000);
  console.log("TRS-TEST", result);
})
  1. Called it from a page with the following template:
```template
{{trsTest(35)}}
```

Code Changes

  1. Added a timeout parameter toweb/syscalls/shell.ts
export function shellSyscalls(client: Client): SysCallMapping {
  return {
    "shell.run": async (
      _ctx,
      cmd: string,
      args: string[],
      timeout: number = 30001, // <---- ADDED THIS
    ): Promise<{ stdout: string; stderr: string; code: number }> => {
      if (!client.httpSpacePrimitives) {
        throw new Error("Not supported in fully local mode");
      }
      const resp = client.httpSpacePrimitives.authenticatedFetch(
        `${client.httpSpacePrimitives.url}/.rpc/shell`,
        {
          method: "POST",
          body: JSON.stringify({
            cmd,
            args,
          }),
        },
        timeout, // <---- ADDED THIS
      );         
  1. Added timeout parameter to plug-api/syscalls/shell.ts
export function run(
  cmd: string,
  args: string[],
  timeout: number, // <---- ADDED THIS
): Promise<{ stdout: string; stderr: string; code: number }> {
  return syscall("shell.run", cmd, args, timeout); // <---- MODIFIED THIS
}

With these changes I got this in the browser console:

TRS-TEST Object { code: 0, stderr: "", stdout: "Starting test script\nSleeping for 35 seconds.\nDone sleeping!\n" }
    ​{
        code: 0,
        stderr: "",
        stdout: "Starting test script\nSleeping for 35 seconds.\nDone sleeping!\n",
        <prototype>: Object { … }
    }

And when debugging via a breakpoint in the function in Code Change 1 I was able to see the 50000 timeout getting passed.

30 seconds is a veritable eternity of a timeout though so I'd be interested in seeing what it is your are running that is taking longer than that and maybe we can speed that up so you don't have to worry about setting a timeout manually that may or may not get hit again.

Anyway like I said I hope that helps, sorry its late so my brain is slower than usual.

Branch with code changes & script

@zefhemel zefhemel added enhancement New feature or request AI implementation candidate Candidate issue to be implemented using AI tools labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI implementation candidate Candidate issue to be implemented using AI tools enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants