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

[WIP] Improve error-handling of mongo shell #1148

Closed
wants to merge 23 commits into from

Conversation

StephenWeatherford
Copy link
Contributor

@StephenWeatherford StephenWeatherford commented Jul 20, 2019

  1. Rewrite interaction with Mongo shell - this especially should provide accurate error messages instead of "timed out" when the "use db" command fails.
  2. Added mongo.shell.args configuration
  3. Some minor cleanup/improvements

Close to finished, sending up the PR early get feedback on the approach. Ideally I'd like to write some tests, too. Rewrote shell.ts completely from scratch after Eric's comments from previous PR.

Fixes #838
Also fixes #1104

Possibly fixes error reporting for:
#1071
#988
#852
#820
#794
#1092

Testcases in OneNote:
image

Copy link
Member

@ejizba ejizba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally a bit wary of all the debounce, setTimeout, etc. code in InteractiveChildProcess (what does await delay(0) even do??) but looks good overall so far. Will wait to do a more thorough review until you say it's ready.

@StephenWeatherford
Copy link
Contributor Author

Generally a bit wary of all the debounce, setTimeout, etc. code in InteractiveChildProcess (what does await delay(0) even do??) but looks good overall so far. Will wait to do a more thorough review until you say it's ready.

Yeah, not happy about it, either, am open to suggestions.

setTimeout(fn, 0) is essentially "execute on next event loop" in Node.js (http://voidcanvas.com/setimmediate-vs-nexttick-vs-settimeout/ - overly complicated). Shouldn't matter what delay I use, I just have to give the stream time to process.

@StephenWeatherford StephenWeatherford deleted the saw/shellerrors6 branch September 3, 2019 19:08
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants