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

Scripts: Not able to import external packages #138

Closed
DivyMohan14 opened this issue Mar 24, 2023 · 5 comments
Closed

Scripts: Not able to import external packages #138

DivyMohan14 opened this issue Mar 24, 2023 · 5 comments

Comments

@DivyMohan14
Copy link
Contributor

DivyMohan14 commented Mar 24, 2023

Really like the way bruno has been structured. Will be looking forward to make this my daily driver for api integrations and testing

Issue Summary:

  • Goal: Wanted to write a Pre Request script which will hit a Login Api get the auth token in the response and will then set the request header (Bearer token) in the request header for the current api.

  • Steps Taken

    1. As the script needed to make an external api call, the idea was to use node-fetch lib.
    2. Followed the steps mentioned in the external libraries docs and completed the entire setup.
  • Issue Faced: I got this error as soon as I ran the script . Cannot find module 'stream' (node-fetch internally uses Stream module )

Screenshot 2023-03-24 at 2 50 18 PM

Possible Issue:

  1. Looking at the problem seems like there is a issue with how the paths are getting resolved.
  2. To debug this, I have setup bruno locally. Looking at the code, looks like bruno internally utilises vm2 module to run the script in a separate container.
  3. So checked out the code for the pre-script part, the issue seems to be in how the env is configured. The context currently is set to 'sandbox', changing it to 'host' or removing it (both of which are the same) resolved the issue for me.

Machine:

  1. Macbook pro (M1 chip)

Also saw one comment related to this on vm2
Wanted to understand why did we use 'sandbox' in the context in bruno?

Will be happy to pick it up, and solve it.

@DivyMohan14 DivyMohan14 changed the title Script: Not able to import external packages Scripts: Not able to import external packages Mar 24, 2023
@helloanoop
Copy link
Contributor

Hi @DivyMohan14 !

Glad that you like Bruno!
Also, nice to see that you are checking out the external libraries feature.
It's one of the features that makes us unique to other similar tools out there.

The reason for using sandbox is from a security standpoint, the goal being yours script should not access filesystem unless you provide explicit access. This is to protect from any malicious 3rd party libs that might try to access stuff using fs module

The long term plan is to give more flexibility for the user to turn off the sandbox, via a ui toggle in the app OR allow to override it using a property in bruno.json at the collection level

For the short term, I feel we can give access to some core nodejs modules inside the sandbox by default.
I am thinking of opening up access to stream, path, url, util modules

Can you take this up @DivyMohan14 and raise a PR ?
You can import the 4 core node modules here and inject them into the sandbox here

There is a problem that you might run into. The pre-request script works fine with synchronous code, but I don't think it yet handles asynchronous code like fetch. Please let me know how it goes?

@helloanoop
Copy link
Contributor

I spent some time on this today.

Added some node core modules to the sandbox. I also plan to expose node-fetch and axios out of the box as this is a very common use case for people to hit a http endpoint pre/post request.

Here is the PR that is tracking this work: https://github.com/usebruno/bruno/pull/139/files

Currently struggling with making async await work in NodeVM
Raised a github issue here: patriksimek/vm2#513

@DivyMohan14
Copy link
Contributor Author

DivyMohan14 commented Mar 28, 2023

Yeah was working on this as well, same thing NodeVM is not waiting for the async function to finish implementation as you already pointed out.

So I thought if this is not fully supported will shift to some sync request libraries like sync-request
This internally uses child_process to spawn a new worker thread and then process the request. Will be trying to set this up and see if I can make this work.

@helloanoop
Copy link
Contributor

@DivyMohan14 Thanks for taking time and making async scripting work !

@karl-gustav
Copy link

@DivyMohan14 Would you mind posting your resulting access-token pre-request script here for posterity?

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

No branches or pull requests

3 participants