Skip to content

Conversation

@mssalemi
Copy link
Contributor

@mssalemi mssalemi commented Apr 14, 2025

Wasm API Support for function-runner

Closes: https://github.com/shop/issues-shopifyvm/issues/32

Function-runner now detects when a module is using the Wasm API by checking if it imports modules with names starting with shopify_function_v.

When a Wasm API function is detected:

  1. The input is automatically transcoded from JSON to MessagePack
  2. The Wasm API provider (shopify_function_v0.0.1.wasm) is loaded
  3. The output is automatically transcoded from MessagePack back to JSON

For output handling:

  1. Locates the actual MessagePack data in the output stream
  2. Extracts only the relevant MessagePack portion for processing
  3. Properly decodes it to JSON for the function result

🎩 Tophat

WASM API

cargo run -- -f tests/fixtures/build/echo.trampolined.wasm -i tests/fixtures/input/wasm_api_function_input.json 

JAVY_V2

cargo run -- -f tests/fixtures/build/js_function_javy_plugin_v2.wasm -i tests/fixtures/input/js_function_input.json -e run

Testing

  • Added unit tests for Wasm API function execution
  • Added integration tests for running Wasm API functions via CLI
  • Tested with the example echo.trampolined.wasm function

note: todo - test everything else is still working as expected

@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from 6658614 to cc411d2 Compare April 14, 2025 14:40
@mssalemi mssalemi marked this pull request as ready for review April 14, 2025 14:42
Copy link
Contributor

@adampetro adampetro left a comment

Choose a reason for hiding this comment

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

I wonder if it would make sense to create a Codec enum like:

enum Codec {
  Json,
  Msgpack,
}

and then select a codec based on conditions, then put all of the conditional codec logic into methods on Codec, such as:

impl Codec {
  fn transcode_from_json_bytes(&self, bytes: Vec<u8>) -> `anyhow::Result<Vec<u8>>` {
    match self {
      Self::Json => Ok(bytes),
      Self::Msgpack => {
        todo!()
      }
    }
  }
}

@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from 3e3ef79 to b0786c0 Compare April 17, 2025 13:45
@mssalemi mssalemi force-pushed the ms.wasm-api-support branch 2 times, most recently from 97e9e7c to f728a5f Compare April 24, 2025 13:47
@saulecabrera
Copy link
Contributor

@mssalemi could you hold on until #464 lands? In that PR I've fixed the codec handling across the entire codebase, which I think will make it easier to do the encoding inferencing introduced in this PR.

@mssalemi mssalemi force-pushed the ms.wasm-api-support branch 2 times, most recently from 5de7479 to a804934 Compare April 25, 2025 17:41
@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from 3258dcb to 7bd4fd0 Compare April 25, 2025 20:06
@mssalemi mssalemi marked this pull request as draft April 28, 2025 12:45
@mssalemi mssalemi marked this pull request as ready for review April 28, 2025 13:10
Copy link
Contributor

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

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

Looking good, I think after the last two comments are resolved we can land this.

@mssalemi mssalemi requested a review from saulecabrera April 29, 2025 15:00
Copy link
Contributor

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

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

Thanks for iterating on this. Approving from my end, I'll leave the final approval to @andrewhassan, given the latest comments.

@mssalemi mssalemi requested a review from andrewhassan May 1, 2025 15:05
Copy link
Contributor

@adampetro adampetro left a comment

Choose a reason for hiding this comment

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

Let's squash the commits before merging

@adampetro
Copy link
Contributor

And hold off on merging for the finalized initial version of the provider module

@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from e9b12e9 to 469de76 Compare May 1, 2025 20:28
@adampetro adampetro force-pushed the ms.wasm-api-support branch from 9bddef5 to 0a99983 Compare May 6, 2025 17:05
@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from a33ddba to d331604 Compare May 7, 2025 12:45
@mssalemi mssalemi force-pushed the ms.wasm-api-support branch from 9a31447 to b0447d9 Compare May 7, 2025 19:20
@mssalemi mssalemi merged commit 620e535 into main May 7, 2025
5 checks passed
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