Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Safely exit by throwing an error#546

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nfischer merged 3 commits intoshelljs:devfromfreitagbr:no-process-exit
Nov 14, 2016
Merged

Safely exit by throwing an error#546

nfischer merged 3 commits intoshelljs:devfromfreitagbr:no-process-exit
Nov 14, 2016

Conversation

@freitagbr
Copy link
Contributor

Fixes#483.

Instead of exiting the process withprocess.exit(1), simply throw an error. This way, the error can be caught by Node itself. If the error is not caught, the error is still printed, and the process exits with a1.

@nfischer
Copy link
Member

Is there a way to make the message be part of the new stack trace? That would be even better.

@nfischernfischer self-assigned thisNov 2, 2016
@freitagbr
Copy link
ContributorAuthor

Do you mean the'ShellJS: internal error' message?

@nfischer
Copy link
Member

Do you mean the 'ShellJS: internal error' message?

Yeah, that's what I meant

@nfischer
Copy link
Member

@freitagbr is it standard to prepend to the message like that?

@freitagbr
Copy link
ContributorAuthor

freitagbr commentedNov 3, 2016
edited
Loading

Probably not, so I will suggest a different option: changing the error name toShellJSInternalError, rather than fiddling with the message itself.

@nfischer
Copy link
Member

@freitagbr that could work

@nfischernfischer added this to thev0.8.0 milestoneNov 5, 2016
@freitagbr
Copy link
ContributorAuthor

I toyed around with the idea of creating an internal error class, but in this instance, I think just changing the name should suffice.

@nfischernfischer changed the base branch frommaster todevNovember 14, 2016 06:13
@nfischer
Copy link
Member

LGTM. Moving this to the dev branch since it's a breaking change.

Hopefully we don't have to worry about unexpected scenarios where you can't assign to properties of exception is caught. This approach should be perfectly fine for any real exception.

@nfischernfischer merged commitbb2075c intoshelljs:devNov 14, 2016
@nfischernfischer added the breakingBreaking change labelNov 14, 2016
@freitagbrfreitagbr deleted the no-process-exit branchNovember 14, 2016 09:27
@nfischer
Copy link
Member

@freitagbr it looks like we may have lost this commit on dev branch with a force push (I wish github supported auto merging). I'll just stick with manual merges instead of rebases to update dev branch.

Would you be able to redo this commit?

@freitagbr
Copy link
ContributorAuthor

Yeah, let me do redo this.

@nfischernfischer mentioned this pull requestJun 7, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

@nfischernfischer

Labels

breakingBreaking change

Projects

None yet

Milestone

v0.8.0

Development

Successfully merging this pull request may close these issues.

2 participants

@freitagbr@nfischer

[8]ページ先頭

©2009-2025 Movatter.jp