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

types: Make Data type-parameter optional in JsonRpcError #102

Merged
merged 3 commits into from
Aug 28, 2023
Merged

types: Make Data type-parameter optional in JsonRpcError #102

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5e053f
types: Make Data type-parameter optional in JsonRpcError
legobeat Aug 23, 2023
df1102a
Expand use of OptionalDataWithOptionalCause
legobeat Aug 23, 2023
e94a4f3
fix/types: make cause actually optional in DataWithOptionalCause
legobeat Aug 24, 2023
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
4 changes: 2 additions & 2 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ module.exports = {
global: {
branches: 94.31,
functions: 94.11,
lines: 97.05,
statements: 97.05,
lines: 97.13,
statements: 97.13,
},
},

Expand Down
9 changes: 6 additions & 3 deletions src/classes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
} from '@metamask/utils';
import safeStringify from 'fast-safe-stringify';

import { DataWithOptionalCause, serializeCause } from './utils';
import type { OptionalDataWithOptionalCause } from './utils';
import { serializeCause } from './utils';

export type { SerializedJsonRpcError };

Expand All @@ -15,7 +16,9 @@ export type { SerializedJsonRpcError };
*
* Permits any integer error code.
*/
export class JsonRpcError<T extends DataWithOptionalCause> extends Error {
export class JsonRpcError<
T extends OptionalDataWithOptionalCause,
> extends Error {
public code: number;

public data?: T;
Expand Down Expand Up @@ -81,7 +84,7 @@ export class JsonRpcError<T extends DataWithOptionalCause> extends Error {
* Permits integer error codes in the [ 1000 <= 4999 ] range.
*/
export class EthereumProviderError<
T extends DataWithOptionalCause,
T extends OptionalDataWithOptionalCause,
> extends JsonRpcError<T> {
/**
* Create an Ethereum Provider JSON-RPC error.
Expand Down
69 changes: 39 additions & 30 deletions src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { JsonRpcError, EthereumProviderError } from './classes';
import { errorCodes } from './error-constants';
import { DataWithOptionalCause, getMessageFromCode } from './utils';
import { OptionalDataWithOptionalCause, getMessageFromCode } from './utils';

type EthereumErrorOptions<T extends DataWithOptionalCause> = {
type EthereumErrorOptions<T extends OptionalDataWithOptionalCause> = {
message?: string;
data?: T;
};

type ServerErrorOptions<T extends DataWithOptionalCause> = {
type ServerErrorOptions<T extends OptionalDataWithOptionalCause> = {
code: number;
} & EthereumErrorOptions<T>;

type CustomErrorArg<T extends DataWithOptionalCause> = ServerErrorOptions<T>;
type CustomErrorArg<T extends OptionalDataWithOptionalCause> =
ServerErrorOptions<T>;

type JsonRpcErrorsArg<T extends DataWithOptionalCause> =
type JsonRpcErrorsArg<T extends OptionalDataWithOptionalCause> =
| EthereumErrorOptions<T>
| string;

Expand All @@ -24,7 +25,7 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
parse: <T extends DataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
parse: <T extends OptionalDataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
getJsonRpcError(errorCodes.rpc.parse, arg),

/**
Expand All @@ -33,7 +34,7 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
invalidRequest: <T extends DataWithOptionalCause>(
invalidRequest: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.invalidRequest, arg),

Expand All @@ -43,16 +44,17 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
invalidParams: <T extends DataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
getJsonRpcError(errorCodes.rpc.invalidParams, arg),
invalidParams: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.invalidParams, arg),

/**
* Get a JSON RPC 2.0 Method Not Found (-32601) error.
*
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
methodNotFound: <T extends DataWithOptionalCause>(
methodNotFound: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.methodNotFound, arg),

Expand All @@ -62,8 +64,9 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
internal: <T extends DataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
getJsonRpcError(errorCodes.rpc.internal, arg),
internal: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.internal, arg),

/**
* Get a JSON RPC 2.0 Server error.
Expand All @@ -73,7 +76,9 @@ export const rpcErrors = {
* @param opts - The error options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
server: <T extends DataWithOptionalCause>(opts: ServerErrorOptions<T>) => {
server: <T extends OptionalDataWithOptionalCause>(
opts: ServerErrorOptions<T>,
) => {
if (!opts || typeof opts !== 'object' || Array.isArray(opts)) {
throw new Error(
'Ethereum RPC Server errors must provide single object argument.',
Expand All @@ -94,16 +99,17 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
invalidInput: <T extends DataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
getJsonRpcError(errorCodes.rpc.invalidInput, arg),
invalidInput: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.invalidInput, arg),

/**
* Get an Ethereum JSON RPC Resource Not Found (-32001) error.
*
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
resourceNotFound: <T extends DataWithOptionalCause>(
resourceNotFound: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.resourceNotFound, arg),

Expand All @@ -113,7 +119,7 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
resourceUnavailable: <T extends DataWithOptionalCause>(
resourceUnavailable: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.resourceUnavailable, arg),

Expand All @@ -123,7 +129,7 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
transactionRejected: <T extends DataWithOptionalCause>(
transactionRejected: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.transactionRejected, arg),

Expand All @@ -133,7 +139,7 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
methodNotSupported: <T extends DataWithOptionalCause>(
methodNotSupported: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.methodNotSupported, arg),

Expand All @@ -143,8 +149,9 @@ export const rpcErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
limitExceeded: <T extends DataWithOptionalCause>(arg?: JsonRpcErrorsArg<T>) =>
getJsonRpcError(errorCodes.rpc.limitExceeded, arg),
limitExceeded: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => getJsonRpcError(errorCodes.rpc.limitExceeded, arg),
};

export const providerErrors = {
Expand All @@ -154,7 +161,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
userRejectedRequest: <T extends DataWithOptionalCause>(
userRejectedRequest: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => {
return getEthProviderError(errorCodes.provider.userRejectedRequest, arg);
Expand All @@ -166,7 +173,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
unauthorized: <T extends DataWithOptionalCause>(
unauthorized: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => {
return getEthProviderError(errorCodes.provider.unauthorized, arg);
Expand All @@ -178,7 +185,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
unsupportedMethod: <T extends DataWithOptionalCause>(
unsupportedMethod: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => {
return getEthProviderError(errorCodes.provider.unsupportedMethod, arg);
Expand All @@ -190,7 +197,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
disconnected: <T extends DataWithOptionalCause>(
disconnected: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => {
return getEthProviderError(errorCodes.provider.disconnected, arg);
Expand All @@ -202,7 +209,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
chainDisconnected: <T extends DataWithOptionalCause>(
chainDisconnected: <T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
) => {
return getEthProviderError(errorCodes.provider.chainDisconnected, arg);
Expand All @@ -214,7 +221,9 @@ export const providerErrors = {
* @param opts - The error options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
custom: <T extends DataWithOptionalCause>(opts: CustomErrorArg<T>) => {
custom: <T extends OptionalDataWithOptionalCause>(
opts: CustomErrorArg<T>,
) => {
if (!opts || typeof opts !== 'object' || Array.isArray(opts)) {
throw new Error(
'Ethereum Provider custom errors must provide single object argument.',
Expand All @@ -237,7 +246,7 @@ export const providerErrors = {
* @param arg - The error message or options bag.
* @returns An instance of the {@link JsonRpcError} class.
*/
function getJsonRpcError<T extends DataWithOptionalCause>(
function getJsonRpcError<T extends OptionalDataWithOptionalCause>(
code: number,
arg?: JsonRpcErrorsArg<T>,
): JsonRpcError<T> {
Expand All @@ -252,7 +261,7 @@ function getJsonRpcError<T extends DataWithOptionalCause>(
* @param arg - The error message or options bag.
* @returns An instance of the {@link EthereumProviderError} class.
*/
function getEthProviderError<T extends DataWithOptionalCause>(
function getEthProviderError<T extends OptionalDataWithOptionalCause>(
code: number,
arg?: JsonRpcErrorsArg<T>,
): EthereumProviderError<T> {
Expand All @@ -270,7 +279,7 @@ function getEthProviderError<T extends DataWithOptionalCause>(
* @param arg - The error message or options bag.
* @returns A tuple containing the error message and optional data.
*/
function parseOpts<T extends DataWithOptionalCause>(
function parseOpts<T extends OptionalDataWithOptionalCause>(
arg?: JsonRpcErrorsArg<T>,
): [message?: string | undefined, data?: T | undefined] {
if (arg) {
Expand Down
8 changes: 8 additions & 0 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ export type DataWithOptionalCause =
cause: unknown;
};

/**
* A data object, that must be either:
*
* - A valid DataWithOptionalCause value.
* - undefined.
*/
export type OptionalDataWithOptionalCause = undefined | DataWithOptionalCause;
Copy link
Member

Choose a reason for hiding this comment

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

Won't these changes cause issues elsewhere because they aren't valid JSON? Goes for both changes in this file

Copy link
Contributor Author

@legobeat legobeat Aug 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The first one should be as fine as it gets, I think.

The one, here, though, I am less confident due to undefined being listed here.

Would it be possible to construct a failure case/regression/validating use-case of some sort? Also, how hard should type-only breakages actually be considered (as they are optional and don't actually affect runtime outcome)?

I'm really struggling to unbreak downstreams in any other way which doesn't cause new detectable inconsistencies and type errors but open to suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

Actually this should be fine I think. We have serializeError for turning the classes into serializable JSON errors, so having undefined in the class is fine.


const FALLBACK_ERROR_CODE = errorCodes.rpc.internal;
const FALLBACK_MESSAGE =
'Unspecified error message. This is a bug, please report it.';
Expand Down