[Feat] New API in Store

[apepkuss]

In this PR, introduce a new API register_plugin_module in Store and a new type alias PluginInstance in plugin mod. In addition, the following changes are also made in this PR:

• Introduce PluginError in wasmedge_types::error mod
• Refactor PluginManager::find and Plugin::mod_instance APIs
• Refactor VmBuilder and Vm

[juntao]

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.

---

Overall, the pull request titled "[Feat] New API in Store" introduces several key changes to the codebase. Here is a summary of the potential issues and most important findings:

1. Potential problems:

   • The change of the return type in the mod_instance method may result in compatibility issues with existing code that relies on the previous return type.
   • It is not clear why the type alias PluginInstance is created and if it is necessary. The code should include comments or explanations to justify its existence.
   • The version of the wasmedge-sdk dependency has been updated to a development version (0.12.0-dev). It is important to ensure that this version is compatible with the rest of the project and does not introduce any breaking changes or regressions.
   • The error handling changes in the find and mod_instance methods should be reviewed for clarity and completeness.
   • The change in the version of rust-sys and wasmedge-sys in the Cargo.toml files requires more context and explanation.

2. Most important findings:

   • Introduction of the PluginInstance type in the plugin module.
   • Addition of the register_plugin_module API function in the Store struct.
   • Introduction of the PluginError type in the wasmedge-types crate.
   • Updates to error handling in the find and mod_instance methods.

In summary, while the changes in the pull request appear to address specific requirements, it is important to address the potential issues and provide more information to ensure compatibility, completeness, and clarity throughout the codebase.

Details

Commit 289b45372fd600c52e8b28963f522117ce6770fa

Key changes:

• A new type PluginInstance is defined in the plugin module.
• The mod_instance method in the Plugin struct now returns Option<PluginInstance> instead of Option<Instance>.
• A type alias PluginInstance is created to refer to Instance.

Potential problems:

• The change of the return type in the mod_instance method may result in compatibility issues with existing code that relies on the previous return type.
• It is not clear why the type alias PluginInstance is created and if it is necessary. The code should include comments or explanations to justify its existence.

Commit 7ab42f45404b106aef22a48a9480640a240c32e7

Key changes:

• Added a new API function register_plugin_module to the Store struct.

Potential problems:

• No potential problems found in the provided code. However, further review may be needed to ensure that the implementation of the register_plugin_module function is correct and does not introduce any issues.

Commit 0437b59796b28aa5767d3d183685b2567dbb6e6a

Key changes:

• The version of the wasmedge-sdk dependency in the Cargo.toml file has been bumped from 0.11.2 to 0.12.0-dev.

Potential problems:

• The version has been updated to a development version (0.12.0-dev). It is important to ensure that this version is compatible with the rest of the project and does not introduce any breaking changes or regressions. It would be beneficial to review the release notes or documentation of the 0.12.0-dev version to understand the changes and assess any potential impact.

Overall, the changes seem to be straightforward, but it is necessary to ensure that the new version is stable and compatible with the existing codebase.

Commit 79b54fd4240337a00cb7250dab766930ea0a77d8

Key changes:

• Introduced a new error type PluginError in the wasmedge-types crate.
• Added variant Plugin(PluginError) to the WasmEdgeError enum.
• Defined the PluginError enum with two variants: Create(String) and Load(String).

Potential problems:

• None observed.

Overall, this patch adds a new error type PluginError to the wasmedge-types crate, which can be used in the WasmEdgeError enum to handle errors related to plugins in the WasmEdge Store. The code changes appear to be correct and do not raise any potential issues.

Commit d7c63241ee60c03fefbcef45c8fb2c870b68f0eb

Key Changes:

• The version of rust-types in the wasmedge-types crate has been bumped from 0.4.3 to 0.4.4 in the Cargo.toml file.

Potential Problems:

• None.

Overall, this patch is a straightforward version bump in the wasmedge-types crate. It does not introduce any potential problems or issues.

Commit 1665560ba8fe3f6d21f309e634843a450f404e9e

Key changes:

• The find method in the PluginManager struct now returns a WasmEdgeResult<Plugin> instead of an Option<Plugin>.
• The mod_instance method in the Plugin struct now returns a WasmEdgeResult<Instance> instead of an Option<Instance>.
• Error handling has been added to the find and mod_instance methods, returning appropriate error types.

Potential problems:

• This patch introduces error handling to the find and mod_instance methods, but it is unclear how the errors are handled by the calling code. It would be helpful to add documentation or comments explaining the intended error handling strategy.
• The error types PluginError::Load and PluginError::Create are used in the error handling, but it is unclear how these errors are defined and what they indicate. It would be helpful to provide more details about these error types and their possible values.
• Without more context, it is difficult to determine if these changes are correct and if they have been adequately tested. It would be useful to have more information about the motivation behind these changes and any relevant test cases.

Commit 31fb6882686bdd79aa2206d9b14812d2f49e2df0

Key Changes:

• The version of the rust-sys crate is being bumped from 0.16.2 to 0.17.0 in the Cargo.toml file.

Potential Problems:

• It is unclear from the provided patch whether the change in version is appropriate and necessary. As a reviewer, it would be worth confirming with the developer why this specific version is being bumped and if it is compatible with the rest of the codebase.
• If this change is being made to fix an issue or add new features, it would be helpful to have more context or a reference to the related GitHub issue or commit.
• It may be beneficial to compare the changes between the two versions (0.16.2 and 0.17.0) of the rust-sys crate to identify any potential breaking changes or dependencies that need to be updated in other parts of the codebase.

Overall, more context and explanation about the version bump and its implications would improve the clarity of this pull request.

Commit 7aab5f40c46829f0804c5d65b05bcbe753227aee

Key changes:

• The Load variant in the PluginError enum has been renamed to NotFound.
• The error message in the NotFound variant has been updated to reflect that the plugin was not found.

Potential problems:

• There doesn't appear to be any potential problems with this change. It is a simple renaming of a variant in an enum and an update to the error message.

Commit ffc7e31c5f981b483d50e7180978108acad8ea48

Key changes:

• The PluginManager::find function has been updated.
• Added a new error variant PluginError::NotFound in the WasmEdgeError::Plugin enum.
• The PluginError::Load variant has been replaced with PluginError::NotFound in the Err result.

Potential problems:

• No potential problems were identified based on the provided patch.

Commit 4be1a2a505252200f38471a1002683184657b97c

Key changes:

• The find method in the PluginManager struct has been updated to return a WasmEdgeResult<Plugin> instead of an Option<Plugin>.
• The mod_instance method in the Plugin struct has been updated to return a WasmEdgeResult<PluginInstance> instead of an Option<PluginInstance>.

Potential problems:

• The patch does not include any changes to the WasmEdgeResult type itself, so it is unclear what kind of error handling is expected in these methods. It would be helpful to have additional context on how the WasmEdgeResult type is used and what specific errors it can return.
• It is also unclear whether the behavior of these methods has changed as a result of the refactoring. The documentation mentions error cases, but it is not clear if these were handled differently before or if there were any changes in the error handling logic.
• The code could benefit from additional unit tests to ensure that the changes to the error handling are working correctly.

Overall, the changes seem to be focused on improving the error handling in the find and mod_instance methods. However, without more context and additional tests, it is difficult to determine if these changes are correct and complete.

Commit e52b47bd061f5410817203e746205fa806f3658d

Key changes:

• The test code in plugin.rs has been updated.

Potential problems:

• The changes in the patch are minimal and seem to be related to updating the assertions in the test code.
• There don't appear to be any major issues or bugs introduced in this patch.

Overall, the changes seem to be relatively safe and do not appear to introduce any problems.

Commit b484fd5a7270c29097222741c9ac7399f4bc869c

Key changes in the patch:

1. The src/vm.rs file has been modified.
2. The VmBuilder struct now has an additional plugin field.
3. The create_plugin_instance method now returns a WasmEdgeResult<PluginInstance> instead of an Option<Instance>. This change allows for handling errors when creating a plugin instance.
4. The Vm struct now uses PluginInstance instead of Instance for plugin host instances.
5. The plugin field in Vm has been changed to a vector of PluginInstance.
6. The auto_detect_plugins method now uses the PluginManager to retrieve plugin names and instances.

Potential problems to address:

1. In the VmBuilder, the code previously used panic! when a plugin instance was not found. It would be better to handle the error gracefully and return a WasmEdgeResult. Consider using an unwrap_or_else or map_or_else to handle the error case.
2. The PluginManager should be properly imported to avoid potential naming conflicts.
3. The PluginInstance type is not defined in the provided code patch, so it's unclear what kind of changes or requirements may exist for this type. It would be helpful to review the code changes in the PluginManager and PluginInstance to ensure compatibility and correctness.
4. It's worth verifying if the modifications to the VmBuilder and Vm structs are backward compatible with the existing code and won't cause any issues elsewhere in the codebase.
5. Review the error handling in the code for any potential error scenarios that may not be handled properly or could lead to unexpected behavior.

Commit f101574dd2e18b8a6ed737e91cd3f65cc253d878

Key changes:

• The version of the wasmedge-sys dependency in the Cargo.toml file has been updated from 0.16 to 0.17.

Potential problems:

• It is not clear why the wasmedge-sys dependency has been updated. The pull request description should provide more context on why this change is necessary.
• There is no information provided on whether the update to version 0.17 of wasmedge-sys is backward compatible or if it introduces any breaking changes. The pull request should include details on any potential compatibility issues and how they have been addressed.

Overall, more information is needed to fully understand the reasoning behind this change and to assess any potential issues introduced by the update.

Commit f6a242f6285540a091e0acf24dd297fea1c252ab

Summary of Key Changes:

• The patch fixes clippy issues in the src/vm.rs file.

Potential Problems:

• None identified.

Overall, the changes in this patch seem to primarily address some clippy issues in the vm.rs file. There are no potential problems identified at this time.

[apepkuss]

@L-jasmine Thanks for the review!