-
Notifications
You must be signed in to change notification settings - Fork 133
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
Catch UncaughtException when serving in non-Markbind directories #2592
Catch UncaughtException when serving in non-Markbind directories #2592
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the good catch! The process is indeed supposed to exit after catching the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice catch! Love the simple fix 👍
@AgentHagu could we take this opportunity to improve the error message, adding something along the line of "this directory does not appear to contain a MarkBind site" |
Agreed with @tlylt; the fix itself atm is good to go, but we can include the error message display in this PR as well @AgentHagu |
I'm thinking we can also include suggestions, for beginner friendliness: e.g. "This directory does not appear to contain a valid MarkBind site. Check that you are running the command in the correct directory!" |
Sounds good! Will adjust the message accordingly |
I have updated the PR to add the suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! A simple but useful fix for those new to MarkBind. Thanks @AgentHagu!
Welcome, @AgentHagu! 🎉 Thank you for your contribution to the MarkBind project! @lhw-1, please remember to add @AgentHagu as an official contributor to our repository. See the full list of contributors here. ✨ |
@lhw-1 Each PR must have a SEMVER impact label, please remember to label the PR properly. |
What is the purpose of this pull request?
Overview of changes:
Fixes #2570
Currently, there is an issue when using the
markbind serve
command in non-Markbind directories, leading to an UncaughtException that is displayed to the user.The problem seems to be due to be a missing
process.exit()
after catching the error. This leads to the code continuing on despite the rootFolder still beingundefined
.The PR also updates the error message to be more user-friendly.
Anything you'd like to highlight/discuss:
This PR only fixes the UncaughtException issue, not the underlying issue mentioned in #1825.
Testing instructions:

Try using the
markbind serve
command in a non-Markbind directory. The UncaughtException should not show. For example:Proposed commit message: (wrap lines at 72 characters)
Catch UncaughtException when serving in non-Markbind directories
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major
,r.Minor
,r.Patch
.Breaking change release note preparation (if applicable):