larsgw / sync-fetch

Synchronous wrapper around the Fetch API
MIT License
38 stars 12 forks source link

Command failed when used in VS Code plugin #23

Open TSMMark opened 2 years ago

TSMMark commented 2 years ago

Error mesage:

Command failed: /Applications/Visual Studio Code.app/Contents/MacOS/Electron XXX/node_modules/sync-fetch/worker.js

Can't figure out why this is happening. Took a look at sync-fetch/worker.js and couldn't make any progress.

TSMMark commented 2 years ago

Possibly related issue https://github.com/microsoft/vscode/issues/144094

larsgw commented 2 years ago

Thank you for the report, I will take a look later. Just to make sure, it runs as expected when using it on MacOS outside of VS Code?

TSMMark commented 2 years ago

yes that's right it works when run on host machine. Possibly related to exec or execsync not being allowed by vs code node, if that's used under the hood?

larsgw commented 2 years ago

I haven't been able to test on a Mac so I'm glad that works. Yes, execFileSync is used here: https://github.com/larsgw/sync-fetch/blob/8ddaaa4c86d62c95cf2f0947f4c927e5b776afd6/index.js#L1. I think most alternatives use it it something similar too, but I might be missing something.

StuartMorris0 commented 2 years ago

Hey @TSMMark did you find another solution for this?

I noticed you had a similar issue on another sync request package. We have a custom eslint rule that uses sync fetching to fetch some data before parsing strings which is not working for the same error above. Running on mac, as an admin user makes no difference here

larsgw commented 2 years ago

Are you also running ESLint from VS Code, or separately?

StuartMorris0 commented 2 years ago

It fails only from VSCode, our build server works fine. Whilst I am sure this is a vscode issue, the issue referenced above appears to be closed for comments and says about running VS as an admin (typically windows).

Still keen to know if @TSMMark was able to find anything else out.

TSMMark commented 2 years ago

I was not able to find a workaround unfortunately. None of the sync fetch implementations work in VS code, and I couldn't figure out how to make a eslint fixer async.

I assume most async fetch packages would work in VS code, but I'm not sure if eslint visitor funcs or fixer funcs can be async... I did try it, but it didn't work. I could not find any docs or info on async funcs in eslint so I'm not sure if it's possible or not

TSMMark commented 2 years ago

@StuartMorris0 if you end up having any success please let me know I'd greatly appreciate it

StuartMorris0 commented 2 years ago

@TSMMark you're correct the eslint rules can't be async, I too went down that rabbit hole for a little while.

I was unable to find a solution, I've disabled our custom eslint plugin for the time being as the sync-fetch error stops vscode eslint from reporting at all :(

I'll monitor the vscode updates in hope it's resolved there sometime.

larsgw commented 2 years ago

There might be an alternative solution for Node v16 and later. I can try to make a development version if you would be interested in testing (otherwise I'll likely wait until Node v14 is EOL)

TSMMark commented 2 years ago

That sounds great, I would definitely be interested in testing it out

larsgw commented 2 years ago

Good news and bad news: it works, takes just as long, and even Node.js v12 supports it. However, I think the only way to implement it is with something of the form while (!isFinished); which takes a lot more CPU than the execSync solution (1103 ms vs 7 ms of CPU time). You can try it at https://github.com/larsgw/sync-fetch/tree/worker_threads but I don't think it's very feasible.

TSMMark commented 2 years ago

It works perfectly! It's unfortunate that it uses more CPU but I'm personally fine with this tradeoff since the alternative is it just not working at all.

Maybe instead of fully replacing the internals with worker threads, it can be can mode which you can opt into via an options object for example. That way users would default to the more performant mode, but have the option if needed?

Thanks for your work on this!

larsgw commented 2 years ago

I am not really sure of the effects of using while (!isFinished);. What you could do, depending on where you want to use this package, is install it directly from GitHub like so:

npm install git://github.com/larsgw/sync-fetch#worker_threads