Skip to content

Commit

Permalink
Merge pull request #82 from polywrap/kris/result-defined-error
Browse files Browse the repository at this point in the history
Changed error type in `Result`to `TError` instead of `TError | undefined`
  • Loading branch information
dOrgJelli committed Oct 31, 2023
2 parents ff66e0a + 593d543 commit 0878b14
Show file tree
Hide file tree
Showing 15 changed files with 32 additions and 35 deletions.
2 changes: 1 addition & 1 deletion packages/client/src/__tests__/core/sanity.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe("sanity", () => {
let config: CoreClientConfig = {
resolver: {
tryResolveUri: (_a: unknown, _b: unknown, _c: unknown) => {
return Promise.resolve(ResultErr());
return Promise.resolve(ResultErr(undefined));
},
},
interfaces: undefined,
Expand Down
28 changes: 14 additions & 14 deletions packages/client/src/__tests__/core/type-test-cases.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { memoryStoragePlugin, ErrResult } from "../helpers";
import { memoryStoragePlugin } from "../helpers";
import { PolywrapClient } from "../../PolywrapClient";

import BigNumber from "bignumber.js";
Expand Down Expand Up @@ -226,7 +226,7 @@ export const typeTestCases = (implementation: string) => {
},
});

method1a = method1a as ErrResult;
if (method1a.ok) throw Error("Expected error, but found `result.ok === true`");
expect(method1a.error).toBeTruthy();
expect(method1a.error?.message).toMatch(
/__wrap_abort: Invalid value for enum 'SanityEnum': 5/gm
Expand All @@ -252,7 +252,7 @@ export const typeTestCases = (implementation: string) => {
},
});

method1c = method1c as ErrResult;
if (method1c.ok) throw Error("Expected error, but found `result.ok === true`");
expect(method1c.error).toBeTruthy();
expect(method1c.error?.message).toMatch(
/__wrap_abort: Invalid key for enum 'SanityEnum': INVALID/gm
Expand All @@ -279,7 +279,7 @@ export const typeTestCases = (implementation: string) => {
arg: 10,
},
});
invalidBoolIntSent = invalidBoolIntSent as ErrResult;
if (invalidBoolIntSent.ok) throw Error("Expected error, but found `result.ok === true`");
expect(invalidBoolIntSent.error).toBeTruthy();
expect(invalidBoolIntSent.error?.message).toMatch(
/Property must be of type 'bool'. Found 'int'./
Expand All @@ -292,7 +292,7 @@ export const typeTestCases = (implementation: string) => {
arg: true,
},
});
invalidIntBoolSent = invalidIntBoolSent as ErrResult;
if (invalidIntBoolSent.ok) throw Error("Expected error, but found `result.ok === true`");
expect(invalidIntBoolSent.error).toBeTruthy();
expect(invalidIntBoolSent.error?.message).toMatch(
/Property must be of type 'int'. Found 'bool'./
Expand All @@ -305,7 +305,7 @@ export const typeTestCases = (implementation: string) => {
arg: [10],
},
});
invalidUIntArraySent = invalidUIntArraySent as ErrResult;
if (invalidUIntArraySent.ok) throw Error("Expected error, but found `result.ok === true`");
expect(invalidUIntArraySent.error).toBeTruthy();
expect(invalidUIntArraySent.error?.message).toMatch(
/Property must be of type 'uint'. Found 'array'./
Expand All @@ -319,7 +319,7 @@ export const typeTestCases = (implementation: string) => {
},
});

invalidBytesFloatSent = invalidBytesFloatSent as ErrResult;
if (invalidBytesFloatSent.ok) throw Error("Expected error, but found `result.ok === true`");
expect(invalidBytesFloatSent.error).toBeTruthy();
expect(invalidBytesFloatSent.error?.message).toMatch(
/Property must be of type 'bytes'. Found 'float64'./
Expand All @@ -335,7 +335,7 @@ export const typeTestCases = (implementation: string) => {
},
});

invalidArrayMapSent = invalidArrayMapSent as ErrResult;
if (invalidArrayMapSent.ok) throw Error("Expected error, but found `result.ok === true`");
expect(invalidArrayMapSent.error).toBeTruthy();
expect(invalidArrayMapSent.error?.message).toMatch(
/Property must be of type 'array'. Found 'map'./
Expand Down Expand Up @@ -446,7 +446,7 @@ export const typeTestCases = (implementation: string) => {
},
});

i8Underflow = i8Underflow as ErrResult;
if (i8Underflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(i8Underflow.error).toBeTruthy();
expect(i8Underflow.error?.message).toMatch(
/integer overflow: value = -129; bits = 8/
Expand All @@ -460,7 +460,7 @@ export const typeTestCases = (implementation: string) => {
second: 10,
},
});
u8Overflow = u8Overflow as ErrResult;
if (u8Overflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(u8Overflow.error).toBeTruthy();
expect(u8Overflow.error?.message).toMatch(
/unsigned integer overflow: value = 256; bits = 8/
Expand All @@ -474,7 +474,7 @@ export const typeTestCases = (implementation: string) => {
second: 10,
},
});
i16Underflow = i16Underflow as ErrResult;
if (i16Underflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(i16Underflow.error).toBeTruthy();
expect(i16Underflow.error?.message).toMatch(
/integer overflow: value = -32769; bits = 16/
Expand All @@ -488,7 +488,7 @@ export const typeTestCases = (implementation: string) => {
second: 10,
},
});
u16Overflow = u16Overflow as ErrResult;
if (u16Overflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(u16Overflow.error).toBeTruthy();
expect(u16Overflow.error?.message).toMatch(
/unsigned integer overflow: value = 65536; bits = 16/
Expand All @@ -502,7 +502,7 @@ export const typeTestCases = (implementation: string) => {
second: 10,
},
});
i32Underflow = i32Underflow as ErrResult;
if (i32Underflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(i32Underflow.error).toBeTruthy();
expect(i32Underflow.error?.message).toMatch(
/integer overflow: value = -2147483649; bits = 32/
Expand All @@ -516,7 +516,7 @@ export const typeTestCases = (implementation: string) => {
second: 10,
},
});
u32Overflow = u32Overflow as ErrResult;
if (u32Overflow.ok) throw Error("Expected error, but found `result.ok === true`");
expect(u32Overflow.error).toBeTruthy();
expect(u32Overflow.error?.message).toMatch(
/unsigned integer overflow: value = 4294967296; bits = 32/
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/__tests__/core/wasm-wrapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { WrapManifest } from "@polywrap/wrap-manifest-types-js";
import { PluginModule, PluginPackage } from "@polywrap/plugin-js";
import { UriResolver } from "@polywrap/uri-resolvers-js";
import { PolywrapClientConfigBuilder } from "@polywrap/client-config-builder-js";
import { mockPluginRegistration, ErrResult } from "../helpers";
import { mockPluginRegistration } from "../helpers";
import { ResultOk } from "@polywrap/result";

jest.setTimeout(200000);
Expand Down Expand Up @@ -175,7 +175,7 @@ describe("wasm-wrapper", () => {
}
);

pluginGetFileResult = pluginGetFileResult as ErrResult;
if (pluginGetFileResult.ok) throw Error("Expected error, but found `result.ok === true`");
expect(pluginGetFileResult.error?.message).toContain(
"client.getFile(...) is not implemented for Plugins."
);
Expand Down
2 changes: 0 additions & 2 deletions packages/client/src/__tests__/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ export const mockPluginRegistration = (uri: string | Uri) => {
};
};

export type ErrResult<E = undefined> = { ok: false; error: E | undefined };

export const incompatiblePlugin = () => {
class IncompatiblePlugin extends PluginModule<Record<string, unknown>> {
async getData(_: {}): Promise<number> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class MockUriResolver implements IUriResolver {
tryResolveUri(
_uri: Uri,
_client: CoreClient
): Promise<Result<UriPackageOrWrapper>> {
): Promise<Result<UriPackageOrWrapper, undefined>> {
throw new Error("Not implemented");
}
}
Expand Down
4 changes: 1 addition & 3 deletions packages/result/src/Result.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
export type Result<T, E = undefined> =
| { ok: true; value: T }
| { ok: false; error: E | undefined };
export type Result<T, E> = { ok: true; value: T } | { ok: false; error: E };
2 changes: 1 addition & 1 deletion packages/result/src/ResultErr.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Result } from "./Result";

// eslint-disable-next-line @typescript-eslint/naming-convention
export const ResultErr = <E>(error?: E): Result<never, E> => {
export const ResultErr = <E>(error: E): Result<never, E> => {
return { ok: false, error };
};
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
[
"wrap://test/error => UriResolverAggregator => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 177, col: 15 })",
"wrap://test/error => UriResolverAggregator => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 178, col: 15 })",
[
"wrap://test/error => StaticResolver - Miss",
"wrap://test/error => ExtendableUriResolver => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 177, col: 15 })",
"wrap://test/error => ExtendableUriResolver => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 178, col: 15 })",
[
"wrap://test/error => ResolverExtension (wrap://package/subinvoke-resolver) => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 177, col: 15 })",
"wrap://test/error => ResolverExtension (wrap://package/subinvoke-resolver) => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 178, col: 15 })",
[
"wrap://package/subinvoke-resolver => Client.loadWrapper => wrapper (wrap://package/subinvoke-resolver)",
[
Expand All @@ -13,7 +13,7 @@
"wrap://package/subinvoke-resolver => StaticResolver - Package (wrap://package/subinvoke-resolver) => package (wrap://package/subinvoke-resolver)"
]
],
"wrap://package/subinvoke-resolver => Client.invokeWrapper => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 177, col: 15 })",
"wrap://package/subinvoke-resolver => Client.invokeWrapper => error (__wrap_abort: Test error\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/test-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 43, col: 21 }\ncode: 51 WRAPPER INVOKE ABORTED\nuri: wrap://package/subinvoke-resolver\nmethod: tryResolveUri\nargs: {\n \"authority\": \"test\",\n \"path\": \"error\"\n} \nsource: { , row: 178, col: 15 })",
[
"wrap://package/test-resolver => Client.loadWrapper => wrapper (wrap://package/test-resolver)",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class SimpleAsyncRedirectResolver implements IUriResolver<Error> {
case "wrap://test/should-redirect":
return new Promise<Result<UriPackageOrWrapper, Error>>((resolve) => {
setTimeout(() => {
const result = UriResolutionResult.ok(
const result = UriResolutionResult.ok<Error>(
new Uri("wrap://test/redirected")
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export abstract class UriResolverAggregatorBase<
}
}

const result = UriResolutionResult.ok(uri);
const result = UriResolutionResult.ok<TResolutionError>(uri);

resolutionContext.trackStep({
sourceUri: uri,
Expand Down
2 changes: 1 addition & 1 deletion packages/uri-resolvers/src/cache/WrapperCacheResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class WrapperCacheResolver<TError>

// Return from cache if available
if (wrapper) {
const result = UriResolutionResult.ok(uri, wrapper);
const result = UriResolutionResult.ok<TError | Error>(uri, wrapper);

resolutionContext.trackStep({
sourceUri: uri,
Expand Down
2 changes: 1 addition & 1 deletion packages/uri-resolvers/src/packages/PackageResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class PackageResolver extends ResolverWithHistory /* $ */ {
*/
protected async _tryResolveUri(
uri: Uri
): Promise<Result<UriPackageOrWrapper>> /* $ */ {
): Promise<Result<UriPackageOrWrapper, never>> /* $ */ {
if (uri.uri !== this._uri.uri) {
return UriResolutionResult.ok(uri);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/uri-resolvers/src/redirects/RedirectResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class RedirectResolver<
*/
protected async _tryResolveUri(
uri: Uri
): Promise<Result<UriPackageOrWrapper>> /* $ */ {
): Promise<Result<UriPackageOrWrapper, never>> /* $ */ {
if (uri.uri !== this.from.uri) {
return UriResolutionResult.ok(uri);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/uri-resolvers/src/wrappers/WrapperResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class WrapperResolver extends ResolverWithHistory /* $ */ {
*/
protected async _tryResolveUri(
uri: Uri
): Promise<Result<UriPackageOrWrapper>> /* $ */ {
): Promise<Result<UriPackageOrWrapper, never>> /* $ */ {
if (uri.uri !== this._uri.uri) {
return UriResolutionResult.ok(uri);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/wasm/src/WasmWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from "@polywrap/core-js";
import { Result, ResultErr, ResultOk } from "@polywrap/result";

// TODO: Remove unused properties
export interface State {
method: string;
args: Uint8Array;
Expand All @@ -41,7 +42,7 @@ export interface State {
error?: string;
args: unknown[];
};
invokeResult?: Result<unknown>;
invokeResult?: Result<unknown, WrapError>;
getImplementationsResult?: Uint8Array;
env: Uint8Array;
resolutionContext?: IUriResolutionContext;
Expand Down

0 comments on commit 0878b14

Please sign in to comment.