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

src: pass along errors from object instantiations (part II) #25822

Closed
wants to merge 4 commits into from

Conversation

@addaleax
Copy link
Member

commented Jan 30, 2019

This addresses the slightly harder-to-address cases of potentially failing NewInstance() calls.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
addaleax added 4 commits Jan 30, 2019
@addaleax

This comment has been minimized.

Copy link
Member Author

commented Feb 1, 2019

@addaleax

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

Any volunteers for review? :)

@addaleax

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2019

@gireeshpunathil

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

sure, will have a look!

@@ -404,19 +410,19 @@ class FileHandle : public AsyncWrap, public StreamBase {
FileHandle& operator=(const FileHandle&&) = delete;

private:
FileHandle(Environment* env, v8::Local<v8::Object> obj, int fd);

// Synchronous close that emits a warning
void Close();
void AfterClose();

class CloseReq : public ReqWrap<uv_fs_t> {
public:
CloseReq(Environment* env,

This comment has been minimized.

Copy link
@gireeshpunathil

gireeshpunathil Feb 8, 2019

Member

any reason why CloseReq does not follow the suit - i.e., a private constructor and a static factory?

This comment has been minimized.

Copy link
@addaleax

addaleax Feb 8, 2019

Author Member

Just that it’s used at only one call site rather than multiple ones. I can add/use a factory constructor if you like?

This comment has been minimized.

Copy link
@gireeshpunathil

gireeshpunathil Feb 8, 2019

Member

ok, I thought so. I think it is fine; doesn't look like anything beneficial of doing so.

@addaleax

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2019

Landed in d999b55...8b79c15

@addaleax addaleax closed this Feb 8, 2019

@addaleax addaleax deleted the addaleax:new-instance-2 branch Feb 8, 2019

addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from tls object creation
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from http2 object creation
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from fs object creations
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from StreamBase req obj creations
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from tls object creation
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from http2 object creation
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from fs object creations
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax added a commit that referenced this pull request Feb 8, 2019
src: pass along errors from StreamBase req obj creations
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@targos targos referenced this pull request Feb 14, 2019
BethGriggs added a commit that referenced this pull request Aug 15, 2019
src: pass along errors from http2 object creation
[This backport applied to v10.x cleanly.]

Backport-PR-URL: #29123
PR-URL: #25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
This was referenced Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.