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

FOGL-7959 Improve error handling for plugin load failure #1142

Merged
merged 22 commits into from
Aug 18, 2023
Merged

Conversation

MarkRiddoch
Copy link
Contributor

Capture some of the reason plugins fail to load and give the user a better indication of the problem
Make the get_plugin_info utility log errors to the error log. This allows users to see missing dependecies when dynamically linking in C++ plugins

@ashish-jabble
Copy link
Member

C based unit tests are failing

00:19:21 CMakeFiles/RunTests.dir/home/ubuntu/Fledge_PR/C/common/config_category.cpp.o: In function `ConfigCategories::ConfigCategories(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
00:19:21 /home/ubuntu/Fledge_PR/C/common/config_category.cpp:49: undefined reference to `StringAround(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int, unsigned int, unsigned int)'
00:19:21 CMakeFiles/RunTests.dir/home/ubuntu/Fledge_PR/C/common/config_category.cpp.o: In function `ConfigCategory::ConfigCategory(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
00:19:21 /home/ubuntu/Fledge_PR/C/common/config_category.cpp:147: undefined reference to `StringAround(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int, unsigned int, unsigned int)'
00:19:21 collect2: error: ld returned 1 exit status
00:19:21 make[2]: *** [RunTests] Error 1
00:19:21 make[1]: *** [CMakeFiles/RunTests.dir/all] Error 2
00:19:21 make: *** [all] Error 2
00:19:21 Tests for ./services/storage/postgres failed

@@ -27,6 +27,9 @@ def get_plugin_info(name, dir):
out, err = p.communicate()
res = out.decode("utf-8")
jdoc = json.loads(res)
except json.decoder.JSONDecodeError as err:
_logger.error("Failed to parse JSON data returned from the plugin information of {}, {} line {} column {}".format(name, err.msg, err.lineno, err.colno))
Copy link
Member

@ashish-jabble ashish-jabble Aug 14, 2023

Choose a reason for hiding this comment

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

No harm to keep specific exception type catch block here. But as we already have multi line stack trace support available at python side; So these type of errors will catch in general exception block and definitely it points to exact problem cause error.

Use multi line stack trace logger style.

_logger.error(err, "Failed to parse JSON data returned from the plugin information of {}.".format(name))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The concept here is to give the user a very clear error message they can pick up on rather than expect them to look at something more general and work out what went wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. Previously, if we have some invalid JSON for a plugin it raises below
ERROR: logger: fledge.services.core.api.utils: json.decoder.JSONDecodeError: Expecting ',' delimiter: line 1 column 2870 (char 2869)

And with the given change
ERROR: logger: fledge.services.core.api.utils: Failed to parse JSON data returned from the plugin information of PLUGIN_NAME, Expecting ',' delimiter line 1 column 2870

So the trailing message still seems to be more developer related logger.

if (access(argv[1], F_OK|R_OK) != 0)
{
fprintf(stderr, "Unable to access library file '%s', exiting...\n", argv[1]);
syslog(LOG_ERR, "Unable to access library file '%s', exiting...\n", argv[1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

May be retain fprintf to stderr also (in addition to syslog) so that errors can be seen on console also.

pintomax
pintomax previously approved these changes Aug 17, 2023
Signed-off-by: Mark Riddoch <[email protected]>
@MarkRiddoch MarkRiddoch merged commit 1633a64 into develop Aug 18, 2023
1 check passed
@MarkRiddoch MarkRiddoch deleted the FOGL-7959 branch August 18, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants