diff options
author | Joyee Cheung <joyeec9h3@gmail.com> | 2018-03-02 19:28:05 +0300 |
---|---|---|
committer | Joyee Cheung <joyeec9h3@gmail.com> | 2018-03-07 20:14:53 +0300 |
commit | 6c25f2ea49c2521dfd2423bf3a06222633ec4dc9 (patch) | |
tree | 4ac455087cbded9f029a37e9e80f4428bf87a6a2 /src/fs_event_wrap.cc | |
parent | 48b5c11b16a4c0035c9beecf252997721dfc2fdd (diff) |
fs: improve errors thrown from fs.watch()
- Add an accessor property `initialized `to FSEventWrap to
check the state of the handle from the JS land
- Introduce ERR_FS_WATCHER_ALREADY_STARTED so calling start()
on a watcher that is already started will throw instead of
doing nothing silently.
- Introduce ERR_FS_WATCHER_NOT_STARTED so calling close()
on a watcher that is already closed will throw instead of
doing nothing silently.
- Validate the filename passed to fs.watch()
- Assert that the handle in the watcher are instances of
FSEvent instead of relying on the illegal invocation error
from the VM.
- Add more assertions in FSEventWrap methods now that we check
`initialized` and the filename in JS land before invoking
the binding.
- Use uvException instead of errornoException to create
the errors with the error numbers from libuv to make them
consistent with other errors in fs.
TODO:
- Improve fs.watchFile() the same way this patch improves fs.watch()
- It seems possible to fire both rename and change event from libuv
together now that we can check if the handle is closed via
`initialized` in JS land.
PR-URL: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Diffstat (limited to 'src/fs_event_wrap.cc')
-rw-r--r-- | src/fs_event_wrap.cc | 73 |
1 files changed, 46 insertions, 27 deletions
diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index 85a09060a11..b3fa3e8d9a0 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -31,6 +31,8 @@ namespace node { using v8::Context; +using v8::DontDelete; +using v8::DontEnum; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; @@ -38,6 +40,9 @@ using v8::Integer; using v8::Local; using v8::MaybeLocal; using v8::Object; +using v8::PropertyAttribute; +using v8::ReadOnly; +using v8::Signature; using v8::String; using v8::Value; @@ -51,7 +56,7 @@ class FSEventWrap: public HandleWrap { static void New(const FunctionCallbackInfo<Value>& args); static void Start(const FunctionCallbackInfo<Value>& args); static void Close(const FunctionCallbackInfo<Value>& args); - + static void GetInitialized(const FunctionCallbackInfo<Value>& args); size_t self_size() const override { return sizeof(*this); } private: @@ -80,6 +85,11 @@ FSEventWrap::~FSEventWrap() { CHECK_EQ(initialized_, false); } +void FSEventWrap::GetInitialized(const FunctionCallbackInfo<Value>& args) { + FSEventWrap* wrap = Unwrap<FSEventWrap>(args.This()); + CHECK(wrap != nullptr); + args.GetReturnValue().Set(wrap->initialized_); +} void FSEventWrap::Initialize(Local<Object> target, Local<Value> unused, @@ -95,6 +105,18 @@ void FSEventWrap::Initialize(Local<Object> target, env->SetProtoMethod(t, "start", Start); env->SetProtoMethod(t, "close", Close); + Local<FunctionTemplate> get_initialized_templ = + FunctionTemplate::New(env->isolate(), + GetInitialized, + env->as_external(), + Signature::New(env->isolate(), t)); + + t->PrototypeTemplate()->SetAccessorProperty( + FIXED_ONE_BYTE_STRING(env->isolate(), "initialized"), + get_initialized_templ, + Local<FunctionTemplate>(), + static_cast<PropertyAttribute>(ReadOnly | DontDelete | v8::DontEnum)); + target->Set(fsevent_string, t->GetFunction()); } @@ -105,22 +127,19 @@ void FSEventWrap::New(const FunctionCallbackInfo<Value>& args) { new FSEventWrap(env, args.This()); } - +// wrap.start(filename, persistent, recursive, encoding) void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) { Environment* env = Environment::GetCurrent(args); - FSEventWrap* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); - if (wrap->initialized_) - return args.GetReturnValue().Set(0); + FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder()); + CHECK_NE(wrap, nullptr); + CHECK(!wrap->initialized_); - static const char kErrMsg[] = "filename must be a string or Buffer"; - if (args.Length() < 1) - return env->ThrowTypeError(kErrMsg); + const int argc = args.Length(); + CHECK_GE(argc, 4); BufferValue path(env->isolate(), args[0]); - if (*path == nullptr) - return env->ThrowTypeError(kErrMsg); + CHECK_NE(*path, nullptr); unsigned int flags = 0; if (args[2]->IsTrue()) @@ -129,19 +148,21 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) { wrap->encoding_ = ParseEncoding(env->isolate(), args[3], kDefaultEncoding); int err = uv_fs_event_init(wrap->env()->event_loop(), &wrap->handle_); - if (err == 0) { - wrap->initialized_ = true; + if (err != 0) { + return args.GetReturnValue().Set(err); + } - err = uv_fs_event_start(&wrap->handle_, OnEvent, *path, flags); + err = uv_fs_event_start(&wrap->handle_, OnEvent, *path, flags); + wrap->initialized_ = true; - if (err == 0) { - // Check for persistent argument - if (!args[1]->IsTrue()) { - uv_unref(reinterpret_cast<uv_handle_t*>(&wrap->handle_)); - } - } else { - FSEventWrap::Close(args); - } + if (err != 0) { + FSEventWrap::Close(args); + return args.GetReturnValue().Set(err); + } + + // Check for persistent argument + if (!args[1]->IsTrue()) { + uv_unref(reinterpret_cast<uv_handle_t*>(&wrap->handle_)); } args.GetReturnValue().Set(err); @@ -209,13 +230,11 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename, void FSEventWrap::Close(const FunctionCallbackInfo<Value>& args) { - FSEventWrap* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); + FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder()); + CHECK_NE(wrap, nullptr); + CHECK(wrap->initialized_); - if (wrap == nullptr || wrap->initialized_ == false) - return; wrap->initialized_ = false; - HandleWrap::Close(args); } |