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.

Sign up for GitHub

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

Jump to bottom

Invocation Flow Improvements (Fixes Encoding Problems) #1000

Merged
merged 4 commits into from
Jun 30, 2022
Merged

Invocation Flow Improvements (Fixes Encoding Problems) #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f83c368
invocation flow + encoding improvements
dOrgJelli Jun 30, 2022
17af67d
optional encoded return value
dOrgJelli Jun 30, 2022
fdcf2b0
fix
dOrgJelli Jun 30, 2022
b307740
lint fix
dOrgJelli Jun 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions packages/js/asyncify/src/AsyncWasmInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ export class AsyncWasmInstance {

private constructor() {}

public static createMemory(config: { module: ArrayBuffer }): WasmMemory {
const bytecode = new Uint8Array(config.module);

public static createMemory(config: { module: Uint8Array }): WasmMemory {
// extract the initial memory page size, as it will
// throw an error if the imported page size differs:
// https://chromium.googlesource.com/v8/v8/+/644556e6ed0e6e4fac2dfabb441439820ec59813/src/wasm/module-instantiate.cc#924
Expand All @@ -73,7 +71,7 @@ export class AsyncWasmInstance {
// 0x__,
]);

const sigIdx = indexOfArray(bytecode, envMemoryImportSignature);
const sigIdx = indexOfArray(config.module, envMemoryImportSignature);

if (sigIdx < 0) {
throw Error(
Expand All @@ -86,7 +84,7 @@ export class AsyncWasmInstance {

// Extract the initial memory page-range size
const memoryInitalLimits =
bytecode[sigIdx + envMemoryImportSignature.length + 1];
config.module[sigIdx + envMemoryImportSignature.length + 1];

if (memoryInitalLimits === undefined) {
throw Error(
Expand All @@ -98,7 +96,7 @@ export class AsyncWasmInstance {
}

public static async createInstance(config: {
module: ArrayBuffer;
module: Uint8Array;
imports: WasmImports;
requiredExports?: readonly string[];
}): Promise<AsyncWasmInstance> {
Expand Down
40 changes: 32 additions & 8 deletions packages/js/client/src/PolywrapClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
InterfaceImplementations,
InvokeOptions,
InvokeResult,
InvokerOptions,
PluginRegistration,
QueryOptions,
QueryResult,
Expand Down Expand Up @@ -47,6 +48,8 @@ import {
JobRunner,
PluginPackage,
RunOptions,
msgpackEncode,
msgpackDecode,
} from "@polywrap/core-js";
import { Tracer } from "@polywrap/tracing-js";

Expand Down Expand Up @@ -197,7 +200,7 @@ export class PolywrapClient implements Client {
public async getFile<TUri extends Uri | string>(
uri: TUri,
options: GetFileOptions
): Promise<string | ArrayBuffer> {
): Promise<string | Uint8Array> {
const wrapper = await this._loadWrapper(this._toUri(uri), options);
const client = contextualizeClient(this, options.contextId);
return await wrapper.getFile(options, client);
Expand Down Expand Up @@ -309,14 +312,14 @@ export class PolywrapClient implements Client {

@Tracer.traceMethod("PolywrapClient: invoke")
public async invoke<TData = unknown, TUri extends Uri | string = string>(
options: InvokeOptions<TUri, PolywrapClientConfig>
options: InvokerOptions<TUri, PolywrapClientConfig>
): Promise<InvokeResult<TData>> {
const { contextId, shouldClearContext } = this._setContext(
options.contextId,
options.config
);

let result: InvokeResult<TData>;
let error: Error | undefined;

try {
const typedOptions: InvokeOptions<Uri> = {
Expand All @@ -327,18 +330,39 @@ export class PolywrapClient implements Client {

const wrapper = await this._loadWrapper(typedOptions.uri, { contextId });

result = (await wrapper.invoke(
const invocableResult = await wrapper.invoke(
typedOptions,
contextualizeClient(this, contextId)
)) as TData;
} catch (error) {
result = { error };
);

if (invocableResult.data !== undefined) {
if (options.encodeResult && !invocableResult.encoded) {
return {
// TODO: if options.encodeResult, fix return type to Uint8Array
data: (msgpackEncode(invocableResult.data) as unknown) as TData,
};
} else if (invocableResult.encoded && !options.encodeResult) {
return {
// TODO: if result.encoded, fix return type to Uint8Array
data: msgpackDecode(invocableResult.data as Uint8Array) as TData,
};
} else {
return {
data: invocableResult.data as TData,
};
}
} else {
error = invocableResult.error;
}
} catch (e) {
error = e;
}

if (shouldClearContext) {
this._clearContext(contextId);
}
return result;

return { error };
}

@Tracer.traceMethod("PolywrapClient: run")
Expand Down
12 changes: 6 additions & 6 deletions packages/js/client/src/__tests__/core/interface-impls.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe("interface-impls", () => {
{
uri: implementation4Uri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [],
Expand Down Expand Up @@ -145,7 +145,7 @@ describe("interface-impls", () => {
{
uri: interface1Uri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [],
Expand All @@ -155,7 +155,7 @@ describe("interface-impls", () => {
{
uri: interface2Uri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [],
Expand Down Expand Up @@ -197,7 +197,7 @@ describe("interface-impls", () => {
{
uri: interfaceUri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [],
Expand Down Expand Up @@ -295,7 +295,7 @@ describe("interface-impls", () => {
{
uri: implementation1Uri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [new Uri(interfaceUri)],
Expand Down Expand Up @@ -330,7 +330,7 @@ describe("interface-impls", () => {
{
uri: implementation1Uri,
plugin: {
factory: () => ({} as PluginModule),
factory: () => ({} as PluginModule<{}>),
manifest: {
schema: "",
implements: [],
Expand Down
2 changes: 1 addition & 1 deletion packages/js/client/src/__tests__/core/resolveUri.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe("resolveUri", () => {
uri: pluginUri.uri,
plugin: {
factory: () => {
return ({} as unknown) as PluginModule;
return ({} as unknown) as PluginModule<{}>;
},
manifest: {
schema: "",
Expand Down
10 changes: 5 additions & 5 deletions packages/js/client/src/__tests__/core/wasm-wrapper.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ describe("wasm-wrapper", () => {
networkNameOrChainId: "testnet",
},
},
noDecode: true,
encodeResult: true,
});

expect(result.error).toBeFalsy();
expect(result.data).toBeTruthy();
expect(result.data instanceof ArrayBuffer).toBeTruthy();
expect(msgpackDecode(result.data as ArrayBuffer)).toContain("0x");
expect(result.data instanceof Uint8Array).toBeTruthy();
expect(msgpackDecode(result.data as Uint8Array)).toContain("0x");
});

it("should invoke wrapper with custom redirects", async () => {
Expand Down Expand Up @@ -291,9 +291,9 @@ describe("wasm-wrapper", () => {
): Int!
`);

const fileBuffer: ArrayBuffer = (await client.getFile(wrapperUri, {
const fileBuffer: Uint8Array = (await client.getFile(wrapperUri, {
path: manifest.schema!,
})) as ArrayBuffer;
})) as Uint8Array;
const decoder = new TextDecoder("utf8");
const text = decoder.decode(fileBuffer);
expect(text).toContain(`getData(
Expand Down
38 changes: 9 additions & 29 deletions packages/js/client/src/plugin/PluginWrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ import {
Client,
GetManifestOptions,
InvokeOptions,
InvokeResult,
InvocableResult,
PluginModule,
PluginPackage,
Uri,
AnyManifestArtifact,
ManifestArtifactType,
GetFileOptions,
Env,
msgpackEncode,
msgpackDecode,
isBuffer,
} from "@polywrap/core-js";
import { Tracer } from "@polywrap/tracing-js";

Expand Down Expand Up @@ -51,15 +51,15 @@ export class PluginWrapper extends Wrapper {
public async getFile(
_options: GetFileOptions,
_client: Client
): Promise<ArrayBuffer | string> {
): Promise<Uint8Array | string> {
throw Error("client.getFile(...) is not implemented for Plugins.");
}

@Tracer.traceMethod("PluginWrapper: invoke")
public async invoke<TData = unknown>(
public async invoke(
options: InvokeOptions<Uri>,
client: Client
): Promise<InvokeResult<TData>> {
): Promise<InvocableResult<unknown>> {
try {
const { method } = options;
const args = options.args || {};
Expand All @@ -79,7 +79,7 @@ export class PluginWrapper extends Wrapper {
let jsArgs: Record<string, unknown>;

// If the args are a msgpack buffer, deserialize it
if (args instanceof ArrayBuffer) {
if (isBuffer(args)) {
const result = msgpackDecode(args);

Tracer.addEvent("msgpack-decoded", result);
Expand All @@ -97,36 +97,16 @@ export class PluginWrapper extends Wrapper {

// Invoke the function
try {
const result = (await module._wrap_invoke(
method,
jsArgs,
client
)) as TData;
const result = await module._wrap_invoke(method, jsArgs, client);

if (result !== undefined) {
const data = result as unknown;

if (process.env.TEST_PLUGIN) {
// try to encode the returned result,
// ensuring it's msgpack compliant
try {
msgpackEncode(data);
} catch (e) {
throw Error(
`TEST_PLUGIN msgpack encode failure.` +
`uri: ${this._uri.uri}\nmodule: ${module}\n` +
`method: ${method}\n` +
`args: ${JSON.stringify(jsArgs, null, 2)}\n` +
`result: ${JSON.stringify(data, null, 2)}\n` +
`exception: ${e}`
);
}
}

Tracer.addEvent("Result", data);

return {
data: data as TData,
data: data,
encoded: false,
};
} else {
return {};
Expand Down
Loading