FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src,stream: remove `*Check*()` calls from non-`Initialize()` functions by RaisinTen · Pull Request #40425 · nodejs/node · GitHub

/ node Public

src,stream: remove *Check*() calls from non-Initialize() functions - #40425

Closed
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:src,stream/remove-Check-calls-from-non-Initialize-functions
ConversationCommits (1)ChecksFiles changed
Closed

src,stream: remove *Check*() calls from non-Initialize() functions#40425
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:src,stream/remove-Check-calls-from-non-Initialize-functions

Conversation

RaisinTen commented Oct 12, 2021

Copy link
Copy Markdown
Member

There is no need to crash the process if any of these checks fail.

Signed-off-by: Darshan Sen darshan.sen@postman.com

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Oct 12, 2021
RaisinTen mentioned this pull request Oct 12, 2021
Closed
RaisinTen requested review from addaleax and jasnell October 12, 2021 10:08
addaleax approved these changes Oct 12, 2021
View reviewed changes
Comment thread src/stream_base-inl.h Outdated
Comment thread src/stream_pipe.cc Outdated
RaisinTen force-pushed the src,stream/remove-Check-calls-from-non-Initialize-functions branch from 636defd to 8e5c7e6 Compare October 12, 2021 14:58
RaisinTen requested a review from addaleax October 12, 2021 14:59
addaleax approved these changes Oct 12, 2021
View reviewed changes
Comment thread src/stream_base.cc Outdated
src,stream: remove *Check*() calls from non-Initialize() functions
bf5fc6b 
There is no need to crash the process if any of these checks fail.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
RaisinTen force-pushed the src,stream/remove-Check-calls-from-non-Initialize-functions branch from 8e5c7e6 to bf5fc6b Compare October 12, 2021 15:50
RaisinTen requested a review from addaleax October 12, 2021 15:52
Mesteery added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Oct 12, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 12, 2021

This comment has been minimized.

Sign in to view
github-actions Bot mentioned this pull request Oct 13, 2021
Open
22 tasks
RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Oct 13, 2021

nodejs-github-bot commented Oct 13, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40269/

This was referenced Oct 14, 2021
Open
Open
Open
Open
Open
Open
RaisinTen added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 20, 2021
github-actions Bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 20, 2021

github-actions Bot commented Oct 20, 2021

Copy link
Copy Markdown
Contributor

Landed in 7ed303b...516cdcd

github-actions Bot closed this Oct 20, 2021
nodejs-github-bot pushed a commit that referenced this pull request Oct 20, 2021
src,stream: remove *Check*() calls from non-Initialize() functions
516cdcd 
There is no need to crash the process if any of these checks fail.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #40425
Reviewed-By: Anna Henningsen <anna@addaleax.net>
RaisinTen deleted the src,stream/remove-Check-calls-from-non-Initialize-functions branch October 21, 2021 04:37
targos pushed a commit that referenced this pull request Oct 23, 2021
src,stream: remove *Check*() calls from non-Initialize() functions
d4b45cc 
There is no need to crash the process if any of these checks fail.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #40425
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos mentioned this pull request Nov 8, 2021
Merged

yulodl commented Nov 23, 2021 •
edited
Loading

Copy link
Copy Markdown

Does this commit sometimes raise error as below

addaleax commented Nov 23, 2021

Copy link
Copy Markdown
Member

@yulodl No, it doesn’t.

BethGriggs pushed a commit that referenced this pull request Nov 23, 2021
src,stream: remove *Check*() calls from non-Initialize() functions
3ac99a2 
There is no need to crash the process if any of these checks fail.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #40425
Reviewed-By: Anna Henningsen <anna@addaleax.net>
BethGriggs mentioned this pull request Nov 26, 2021
Merged
1 task
RaisinTen mentioned this pull request Feb 21, 2022
Merged
RaisinTen added a commit to RaisinTen/node that referenced this pull request May 29, 2022
src: convey potential exceptions during StreamPipe construction
98adc6c 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: nodejs#40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>
RaisinTen mentioned this pull request May 29, 2022
Merged
nodejs-github-bot pushed a commit that referenced this pull request Jun 2, 2022
src: convey potential exceptions during StreamPipe construction
1496d43 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: #40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
src: convey potential exceptions during StreamPipe construction
ad77183 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: #40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
src: convey potential exceptions during StreamPipe construction
5e65c1f 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: #40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
src: convey potential exceptions during StreamPipe construction
ca8ef7c 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: #40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
src: convey potential exceptions during StreamPipe construction
c82d992 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: #40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
src: convey potential exceptions during StreamPipe construction
fd40de1 
This moves the V8 calls during the StreamPipe construction to an
overload of StreamPipe::New(), so that it is possible to indicate if
there is a pending exception/termination.

Refs: nodejs/node#40425 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#43240
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
jasnell Awaiting requested review from jasnell

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL