-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[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.
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
[OV JS] Fix hardcoded error msg #25272
Conversation
Co-authored-by: Vishniakov Nikolai <[email protected]>
| }; | ||
|
|
||
| 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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
### Details: - Each error msg concerned the 'compiledModelSync' method. - A new parameter with method name was added to `get_parameters_error_msg` --------- Co-authored-by: Vishniakov Nikolai <[email protected]>
Details:
get_parameters_error_msg