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

src: only call FatalException if not verbose #12826

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@danbev
Member

danbev commented May 4, 2017

This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

@jasnell

This comment has been minimized.

Show comment
Hide comment
Member

jasnell commented May 4, 2017

@cjihrig

cjihrig approved these changes May 4, 2017

Second commit LGTM

@danbev

This comment has been minimized.

Show comment
Hide comment
@addaleax

For my understanding: There isn’t any behavioural change because in the verbose case V8 will call OnMessage, right? If so, LGTM.

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev May 9, 2017

Member

For my understanding: There isn’t any behavioural change because in the verbose case V8 will call OnMessage, right? If so, LGTM.

Yes that is my understanding as well, if a TryCatch is used and set to verbose, the handler will be called.

Member

danbev commented May 9, 2017

For my understanding: There isn’t any behavioural change because in the verbose case V8 will call OnMessage, right? If so, LGTM.

Yes that is my understanding as well, if a TryCatch is used and set to verbose, the handler will be called.

@bnoordhuis

LGTM if you bump the patch level in v8-version.h. Did you request a back-port to the 5.8 branch?

danbev added some commits May 2, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}
src: only call FatalException if not verbose
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.
@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev May 15, 2017

Member

if you bump the patch level in v8-version.h

I've bumped the patch level now.

Did you request a back-port to the 5.8 branch?

I've not done so but would be happy to. Just to clarify, you mean create back-port this to 5.8 in V8 right? I've never done that before so if you have any pointers about how to go about doing that (what branch would I base the change set on)?

Member

danbev commented May 15, 2017

if you bump the patch level in v8-version.h

I've bumped the patch level now.

Did you request a back-port to the 5.8 branch?

I've not done so but would be happy to. Just to clarify, you mean create back-port this to 5.8 in V8 right? I've never done that before so if you have any pointers about how to go about doing that (what branch would I base the change set on)?

danbev added a commit to danbev/node that referenced this pull request May 16, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

danbev added a commit to danbev/node that referenced this pull request May 16, 2017

src: only call FatalException if not verbose
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

PR-URL: nodejs#12826
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev May 16, 2017

Member

Landed in fefab90 and 986e1d2

Member

danbev commented May 16, 2017

Landed in fefab90 and 986e1d2

@danbev danbev closed this May 16, 2017

@danbev danbev deleted the danbev:is-verbose-todo branch May 16, 2017

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis May 16, 2017

Member

@danbev Just so you know, the Author tag is "daniel.bevenius" in commit 986e1d2. git am-d from upstream, I assume?

Member

bnoordhuis commented May 16, 2017

@danbev Just so you know, the Author tag is "daniel.bevenius" in commit 986e1d2. git am-d from upstream, I assume?

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev May 17, 2017

Member

@bnoordhuis Thanks, I forgot to change it. I'll see if I can change my name in V8 so they are the same just in case I do this again and forget.
While I have you there would you be able to comment about my question above regarding the backporting?

Member

danbev commented May 17, 2017

@bnoordhuis Thanks, I forgot to change it. I'll see if I can change my name in V8 so they are the same just in case I do this again and forget.
While I have you there would you be able to comment about my question above regarding the backporting?

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev May 17, 2017

Member

@bnoordhuis perfect, thanks!

Member

danbev commented May 17, 2017

@bnoordhuis perfect, thanks!

@danbev

This comment has been minimized.

Show comment
Hide comment
Member

danbev commented May 18, 2017

anchnk added a commit to anchnk/node that referenced this pull request May 19, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

anchnk added a commit to anchnk/node that referenced this pull request May 19, 2017

src: only call FatalException if not verbose
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

PR-URL: nodejs#12826
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@jasnell jasnell referenced this pull request May 28, 2017

Closed

8.0.0 Release Proposal #12220

targos added a commit to targos/node that referenced this pull request Jun 6, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

targos added a commit to targos/node that referenced this pull request Jun 7, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@gibfahn gibfahn referenced this pull request Jun 15, 2017

Closed

Auditing for 6.11.1 #230

2 of 3 tasks complete

addaleax added a commit to targos/node that referenced this pull request Jun 17, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

addaleax added a commit that referenced this pull request Jun 17, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@addaleax addaleax referenced this pull request Jun 17, 2017

Merged

v8.2.0 proposal #13744

addaleax added a commit that referenced this pull request Jun 21, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Jun 22, 2017

Member

I'm a bit confused as to why 986e1d2 is showing up in both the v8.0.0 proposal as well as the v8.2.0 proposal. It would appear that only one of the commits from this PR ended up making its way to the 8.0.0 release which weird me out

/cc @jasnell @addaleax

Member

MylesBorins commented Jun 22, 2017

I'm a bit confused as to why 986e1d2 is showing up in both the v8.0.0 proposal as well as the v8.2.0 proposal. It would appear that only one of the commits from this PR ended up making its way to the 8.0.0 release which weird me out

/cc @jasnell @addaleax

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Jun 22, 2017

Member

@MylesBorins The commit you linked was being re-applied as part of the V8 5.9 upgrade, but both were part of 8.0.0 … does that make sense?

Member

addaleax commented Jun 22, 2017

@MylesBorins The commit you linked was being re-applied as part of the V8 5.9 upgrade, but both were part of 8.0.0 … does that make sense?

addaleax added a commit that referenced this pull request Jun 24, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg added a commit that referenced this pull request Jun 29, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

addaleax added a commit that referenced this pull request Jul 11, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Jul 17, 2017

Member

Should this be backported to v6.x?

Member

MylesBorins commented Jul 17, 2017

Should this be backported to v6.x?

addaleax added a commit that referenced this pull request Jul 18, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

targos added a commit to targos/node that referenced this pull request Jul 21, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

addaleax added a commit that referenced this pull request Jul 24, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@addaleax addaleax referenced this pull request Jul 24, 2017

Closed

v8.3.0 proposal #14452

MylesBorins added a commit that referenced this pull request Aug 14, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Aug 14, 2017

src: only call FatalException if not verbose
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

PR-URL: #12826
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Aug 16, 2017

deps: cherry-pick f5fad6d from upstream v8
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{#45018}

PR-URL: #12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Aug 16, 2017

src: only call FatalException if not verbose
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

PR-URL: #12826
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@MylesBorins MylesBorins referenced this pull request Aug 16, 2017

Merged

v6.11.3-proposal #14852

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment