Skip to content

[OV JS] Fix hardcoded error msg #25272

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.

Sign up for GitHub

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

Merged
almilosz merged 4 commits into from
Jul 1, 2024
Merged

[OV JS] Fix hardcoded error msg #25272

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
528923b
Fix hardcoded part
almilosz Jun 28, 2024
248126e
Update src/bindings/js/node/tests/read_model.test.js
almilosz Jun 28, 2024
6495e02
Move js name of method to ov throw
almilosz Jul 1, 2024
debb3bd
Merge branch 'almilosz/fix/error-msg' of https://github.com/almilosz/…
almilosz Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/bindings/js/node/include/type_validation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ bool validate(const Napi::CallbackInfo& info, std::vector<std::string>& allowed_
return InputParameters<Ts...>::validate(info, std::index_sequence_for<Ts...>{});
};

std::string get_parameters_error_msg(const Napi::CallbackInfo& info, std::vector<std::string>& allowed_signatures);
std::string get_parameters_error_msg(const Napi::CallbackInfo& info,
Copy link
Contributor

@praasz praasz Jun 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to consider if this function is not to specialized instead build string for signatures it do more.
What if name is not required or there will be need something else.

Maybe leave this function unchanged to build signature string and use assert macro to build final message where required.

OPENVINO_THROW("Method 'my_method' called with invalid parameters\n", ov::js::get_parameters_signature_str(info, allowed_signatures));

Macro build message with exact location (file, line), so maybe give method name is not required at all.

Copy link
Contributor Author

@almilosz almilosz Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact location points to C++ implementation of method that may have a different name in JS API.
Here it's readModelSync vs read_model_sync, but in some cases it might be quite confusing for a user.

std::string name,
std::vector<std::string>& allowed_signatures);
} // namespace js
} // namespace ov
4 changes: 2 additions & 2 deletions src/bindings/js/node/src/core_wrap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Napi::Value CoreWrap::read_model_sync(const Napi::CallbackInfo& info) {
} else if (ov::js::validate<Napi::String>(info, allowed_signatures)) {
model = _core.read_model(info[0].ToString());
} else {
const auto error_message = ov::js::get_parameters_error_msg(info, allowed_signatures);
const auto error_message = ov::js::get_parameters_error_msg(info, "readModelSync", allowed_signatures);

OPENVINO_THROW(error_message);
}
Expand Down Expand Up @@ -169,7 +169,7 @@ Napi::Value CoreWrap::compile_model_sync_dispatch(const Napi::CallbackInfo& info
return compile_model_sync(info, info[0].ToObject(), info[1].ToString(), config);
}

const auto error_message = ov::js::get_parameters_error_msg(info, allowed_signatures);
const auto error_message = ov::js::get_parameters_error_msg(info, "compileModelSync", allowed_signatures);

OPENVINO_THROW(error_message);
} catch (std::exception& e) {
Expand Down
9 changes: 6 additions & 3 deletions src/bindings/js/node/src/type_validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,12 @@ bool validate_value<TensorWrap>(const Napi::Env& env, const Napi::Value& value)
return value.ToObject().InstanceOf(prototype.Value().As<Napi::Function>());
}

std::string get_parameters_error_msg(const Napi::CallbackInfo& info, std::vector<std::string>& allowed_signatures) {
return "Method 'compileModelSync' called with incorrect parameters.\nProvided signature: " +
js::get_current_signature(info) + " \nAllowed signatures:\n- " + ov::util::join(allowed_signatures, "\n- ");
std::string get_parameters_error_msg(const Napi::CallbackInfo& info,
std::string name,
std::vector<std::string>& allowed_signatures) {
return "Method '" + name +
"' called with incorrect parameters.\nProvided signature: " + js::get_current_signature(info) +
" \nAllowed signatures:\n- " + ov::util::join(allowed_signatures, "\n- ");
}
} // namespace js
} // namespace ov
7 changes: 7 additions & 0 deletions src/bindings/js/node/tests/read_model.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ describe('Core.readModeSync', () => {
assert.equal(model.inputs.length, 1);
});

it('readModeSync throws', () => {
assert.throws(
() => core.readModelSync(core),
/Method 'readModelSync' called with incorrect parameters./,
)
});

it('readModeSync(modelUint8ArrayBuffer, weightsUint8ArrayBuffer) ', () => {
const model = core.readModelSync(
new Uint8Array(modelFile.buffer),
Expand Down