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

[bug] pipe crashing at deno runtime level ($200) #445

Closed
louis030195 opened this issue Oct 8, 2024 · 22 comments
Closed

[bug] pipe crashing at deno runtime level ($200) #445

louis030195 opened this issue Oct 8, 2024 · 22 comments
Labels
💎 Bounty bug Something isn't working 💰 Rewarded

Comments

@louis030195
Copy link
Collaborator

denoland/deno_core#898

/bounty 200

definition of done:

  • does not crash anymore
@louis030195 louis030195 added the bug Something isn't working label Oct 8, 2024
Copy link

linear bot commented Oct 8, 2024

Copy link

algora-pbc bot commented Oct 8, 2024

💎 $200 bounty • Screenpi.pe

💎 $100 bounty • Screenpi.pe

Steps to solve:

  1. Start working: Comment /attempt #445 with your implementation plan
  2. Submit work: Create a pull request including /claim #445 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to mediar-ai/screenpipe!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟢 @ologbonowiwi Oct 8, 2024, 10:09:21 PM #445
🟢 @jandremarais Oct 10, 2024, 7:26:30 AM WIP

@ologbonowiwi
Copy link
Contributor

Hi @louis030195, I've taken a look at the deno/rust files, and I'm wondering why everything is set up on rust instead of letting the deno file run the parallelism. They have the Thread package, which probably should be more deterministic here. What do you think?

@louis030195
Copy link
Collaborator Author

@ologbonowiwi not familiar with deno, it's my first time using it, all i want ideally is:

  • dev can write pipes (plugins) in ts/js
  • can use dependencies do for example bun i ai, use normal import, all normal code and not weird deno import
  • can write some abstraction/sdk to facilitate getting screenpipe data into LLMs

atm what does not work:

  • cannot use dependencies
  • deno crashes sometimes

the thread package seems interesting but i'd prefer people can just write normal js without problem and since most people these days use chatgpt to write code, it unlikely will use thread package (maybe im wrong)

@ologbonowiwi
Copy link
Contributor

ologbonowiwi commented Oct 8, 2024

ideally we could have common js code -> run through through our deno code that uses the Thread package (and therefore can be parallelized without the hurdle of users doing it themselves)

It’s my first time looking into an issue on the project so I’m not sure how it really works. any place where I could learn more about this part of the project? I need to gain more context on this but I have a good knowledge on js side so if I fill that gap I can handle this issue. /attempt #445

Algora profile Completed bounties Tech Active attempts Options
@ologbonowiwi 51 bounties from 6 projects
Rust, TypeScript,
Shell & more
Cancel attempt

@louis030195
Copy link
Collaborator Author

https://github.com/mediar-ai/screenpipe/blob/main/screenpipe-core/src/pipes.rs

and the deno folder

these are the only deno related code

@ologbonowiwi
Copy link
Contributor

ologbonowiwi commented Oct 10, 2024

I heard that Deno has significant challenges making its runtime compatible with Node. I think there's a high chance we’re missing something Deno would natively do in case we use their CLI (or even starting their runtime in Rust, but not doing the back-and-forth to “mock” things like filesystem and fetch calls).

Besides, we were not leveraging the event loop's power to do asynchronous processing (I/O, network calls, and so on). We’d be better off relying on the already-established tools of the JS runtime/ecosystem to do this heavy work and only having a specific layer inject some dependencies, such as custom pipes.

Besides that, I think the way it is currently done has a high risk of unexpected behavior. If I’m using fetch, I expect it to behave as it has been in browsers forever. But since we're using reqwest under the hood, the original assumption of someone used to the actual fetch from chrome/node/deno is wrong, which can bring debug issues for ppl who need to create pipes in the future.

I'd recommend reframing our approach to rely most on the deno runtime and letting it do its magic on running the Js/ts scripts or creating explicitly different helpers. I mean, if we want to keep the filesystem requests on rust-land, per se, we could export pipe.writeFile instead of having a “fake” fs.writeFile, which does not rely on libuv as most JavaScript runtimes do. Besides that, with the deno, I believe we’d have dependencies importing out of the box.

I'm refraining from working on this because I think the way we're handling the scripts running is the root cause of our issues, and fixing it is more complex than I anticipated.

@jandremarais
Copy link
Contributor

jandremarais commented Oct 10, 2024

I can't speak much to the above but I know that BorrowMutError is a consequence of using deno in tokio's multi-thread runtime. Deno needs to a in a current-thread runtime. I can make an /attempt #445, but the tricky part is going to be how to decide how to mix it in with the existing multi-threaded runtime.

@ologbonowiwi
Copy link
Contributor

but the tricky part is going to be how to decide how to mix it in with the existing multi-threaded runtime

Yeah, I think we can have issues with the fact we are relying on Tokio threads to do async stuff such as filesystem calls.

imo, we'd be better off handing this to deno (cause they already support it anyways)

@louis030195
Copy link
Collaborator Author

I heard that Deno has significant challenges making its runtime compatible with Node. I think there's a high chance we’re missing something Deno would natively do in case we use their CLI (or even starting their runtime in Rust, but not doing the back-and-forth to “mock” things like filesystem and fetch calls).

Besides, we were not leveraging the event loop's power to do asynchronous processing (I/O, network calls, and so on). We’d be better off relying on the already-established tools of the JS runtime/ecosystem to do this heavy work and only having a specific layer inject some dependencies, such as custom pipes.

Besides that, I think the way it is currently done has a high risk of unexpected behavior. If I’m using fetch, I expect it to behave as it has been in browsers forever. But since we're using reqwest under the hood, the original assumption of someone used to the actual fetch from chrome/node/deno is wrong, which can bring debug issues for ppl who need to create pipes in the future.

I'd recommend reframing our approach to rely most on the deno runtime and letting it do its magic on running the Js/ts scripts or creating explicitly different helpers. I mean, if we want to keep the filesystem requests on rust-land, per se, we could export pipe.writeFile instead of having a “fake” fs.writeFile, which does not rely on libuv as most JavaScript runtimes do. Besides that, with the deno, I believe we’d have dependencies importing out of the box.

I'm refraining from working on this because I think the way we're handling the scripts running is the root cause of our issues, and fixing it is more complex than I anticipated.

do you mean something like

multiple deno projects in a multi-folder setup with a single cli command. here's how you can set it up:

  1. create a root directory for your projects
  2. add subdirectories for each project
  3. create a main script to import and run all projects

here's a quick example structure:

root/
├── project1/
│   └── main.ts
├── project2/
│   └── main.ts
├── project3/
│   └── main.ts
└── run_all.ts

in your run_all.ts file, you can import and run all the projects:

import "./project1/main.ts";
import "./project2/main.ts";
import "./project3/main.ts";

console.log("all projects started");

then, you can run everything with a single command:

deno run --allow-read --allow-net run_all.ts

make sure to include any necessary permissions flags (like --allow-read or --allow-net) that your projects might need.

pro tip: if your projects are more complex and need to be initialized or have async operations, you could modify run_all.ts to handle that:

import { start as start1 } from "./project1/main.ts";
import { start as start2 } from "./project2/main.ts";
import { start as start3 } from "./project3/main.ts";

async function runAll() {
  await Promise.all([
    start1(),
    start2(),
    start3(),
  ]);
  console.log("all projects started");
}

runAll();

eg.

rust -> run deno cli that run some js code that import all pipes and run them

or (AI suggestions):

Based on the provided code for pipes.rs and pipe_manager.rs, here are 10 high-level ways you could host pipes using Deno:

  1. Deno Deploy:
    Use Deno's serverless platform to host each pipe as a separate serverless function.

  2. Deno HTTP Server:
    Create a simple HTTP server using Deno's standard library to serve pipes.

  3. Oak Framework:
    Utilize the Oak middleware framework for Deno to create a more robust API for serving pipes.

  4. WebSocket Server:
    Implement a WebSocket server in Deno to allow real-time communication with pipes.

  5. Deno Worker Threads:
    Run each pipe in a separate Deno worker thread for improved concurrency.

  6. Deno Plugins:
    Develop Deno plugins to extend functionality and host pipes as loadable modules.

  7. Deno with Docker:
    Containerize your Deno application and pipes for easy deployment and scaling.

  8. Deno CLI Tool:
    Create a CLI tool using Deno that can run pipes locally or on remote servers.

  9. Deno with Kubernetes:
    Deploy your Deno application and pipes on a Kubernetes cluster for orchestration.

  10. Deno Static File Server:
    Use Deno's file server capabilities to serve static pipe files and execute them on demand.

To implement any of these approaches, you'd need to modify your existing code to work with Deno instead of the current runtime. This would involve adapting the module system, using Deno-specific APIs, and potentially restructuring parts of your application.

@ologbonowiwi
Copy link
Contributor

ologbonowiwi commented Oct 10, 2024

rust -> run deno cli that runs some js code that imports all pipes and runs them

That's precisely it. deno is mostly Node-compatible (source: Node APIs). So we can have imports + node native support + deno native support almost effortlessly.

It can keep the Rust layer that instantiates the Deno runtime. We then run the run_all.ts file, which can receive some arguments (such as the pipes it needs to execute).
Alternatively, we can somehow handle this to deno cli and mention to the users that they can run pipes but need to install deno first (or IDK if we can have deno in the project or use it as cli)

@louis030195
Copy link
Collaborator Author

rust -> run deno cli that runs some js code that imports all pipes and runs them

That's precisely it. deno is mostly Node-compatible (source: Node APIs). So we can have imports + node native support + deno native support almost effortlessly.

It can keep the Rust layer that instantiates the Deno runtime. We then run the run_all.ts file, which can receive some arguments (such as the pipes it needs to execute). Alternatively, we can somehow handle this to deno cli and mention to the users that they can run pipes but need to install deno first (or IDK if we can have deno in the project or use it as cli)

i see

  • so next step i think of is that i will do a large refactor to make pipes imported by some js/ts core module and execute them
  • this module will be execxuted by deno cli itself
  • deno cli will be installed in in pre_build.js for the app on all OSes but CLI users need to install deno cli

this will:

  • solve the crash here
  • remove need to reimplement all the node api in rust myself
  • allow dependencies
  • let people just write normal js and it works
  • let people run servers or whatever and it works

@louis030195
Copy link
Collaborator Author

update: afraid not going to the deno cli path: https://github.com/mediar-ai/screenpipe/pull/462/files

problem is that you can't write rust extensions easily this way and big next feature we're launching is the ability to control your computer in js eg

await pipe.click({x: 200, y: 350})

await pipe.type("yo")

// or provide LLM tools like typing and mouse and give full autonomy to solve tasks on the computer ... 

right now i am going to investigate if i can import packages or use deno.json like when using cli using the current way

@louis030195
Copy link
Collaborator Author

other info: https://github.com/denoland/deno/blob/main/ext/webgpu/lib.rs

seems to be possible to write rust extensions

although not sure if need to be web assembly compatible (dont think the extensions i want to build can compile to wasm)

@louis030195
Copy link
Collaborator Author

louis030195 commented Oct 10, 2024

ok here are my updated thoughts:

  • i will use deno cli because i think that's where the deno team put the most effort and is more reliable and clean
  • i will implement short term hacks to be able to use low level api from the pipes like: opening http endpoint in screenpipe cli to control mouse/keyboard (that gives some Big Brother type ideas like employer controlling your computer XD) until i figure out cleaner way to create deno extensions in rust

@ologbonowiwi
Copy link
Contributor

ologbonowiwi commented Oct 10, 2024

just looked at #462. it looks way cleaner now. did fetch and the other async stuff you used work properly?

louis030195 added a commit that referenced this issue Oct 11, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
feat: use dependencies in pipes #445
@louis030195
Copy link
Collaborator Author

now everything works

except idk how to write rust extensions yet which is useful for low level / native stuff like keyboard control or mouse

@ologbonowiwi
Copy link
Contributor

great to know!

we can maybe close this issue then? And maybe create another issue for supporting rust?

PS: I would appreciate a tip if my suggestions helped fix this.

@louis030195
Copy link
Collaborator Author

/tip $100 @ologbonowiwi

thank you for the suggestions 🙏

Copy link

algora-pbc bot commented Oct 11, 2024

Copy link

algora-pbc bot commented Oct 11, 2024

@ologbonowiwi: You just got a $100 tip! We'll notify you once it is processed.

Copy link

algora-pbc bot commented Oct 24, 2024

🎉🎈 @ologbonowiwi has been awarded $100! 🎈🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working 💰 Rewarded
Projects
None yet
Development

No branches or pull requests

3 participants