Merge callback instance imports#1998
Merge callback instance imports#1998cursor[bot] wants to merge 1 commit intofeat/sync-wasm-merge-callbackfrom
Conversation
The create_merge_callback() function was using wasmer::Imports::default() which is empty. However, real WASM modules import host functions from the 'env' namespace. Even though merge functions don't actually call those host functions at runtime, Wasmer requires all imports to be satisfied at instantiation time. This fix creates stub imports that satisfy the module's import requirements. These stubs trap if called (which shouldn't happen since merge functions are pure data merge operations).
|
Cursor Agent can help with this pull request. Just |
|
Your PR title does not adhere to the Conventional Commits convention: Common errors to avoid:
|
![meroreviewer[bot]](https://avatars.githubusercontent.com/in/2793509?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Code Reviewer
Reviewed by 3 agents | Quality score: 100% | Review time: 102.6s
🟡 1 warnings. See inline comments.
🤖 Generated by AI Code Reviewer | Review ID: review-1e5e7c0a
| let instance = Instance::new(&mut store, &self.module, &imports).ok()?; | ||
| merge_callback::RuntimeMergeCallback::from_instance(store, instance) | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 Bug fix lacks regression test
This fixes a bug where modules with host imports couldn't instantiate merge callbacks, but no test was added to prevent regression.
Suggested fix:
Add a test with a WASM module that declares host function imports and verify `create_merge_callback()` returns `Some(_)` instead of `None`.
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
1 participant
Fix: Merge callback instance instantiation fails due to missing imports
Description
This PR addresses a bug in the
create_merge_callback()function where WASM modules for custom CRDT merge logic failed to instantiate. The issue stemmed from usingwasmer::Imports::default()(empty imports), which caused instantiation to fail for modules that declare host function imports (common in real applications), even if the merge export itself doesn't call them. This resulted in the merge callback beingNoneandNoWasmCallbackerrors during conflict resolution forCrdtType::Custom.The fix ensures that the merge callback instance is created with a complete set of stub imports, satisfying all module import requirements. These stubs will trap if called, but pure merge functions are not expected to call host functions.
Fixes bug
b4b3194a-aa72-4c72-a538-e473373df5b1.Test plan
cargo check.cargo test, and all 107 tests passed.cargo fmt --checkandcargo clippyto ensure code quality and style.Documentation update
None.