coinbase / coinbase/onchainkit

Feature Request: rm APIError type from each JSON-RPC request

Open
#2,431 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
1k
Forks
520
Avg merge
32m
Merged PRs (30d)
2

Description

### Describe the solution you'd like

For example, `BuildSwapTransactionResponse` is returned type from `buildSwapTransaction`

```typescript

export type BuildSwapTransactionResponse = BuildSwapTransaction | APIError;

export async function buildSwapTransaction(
params: BuildSwapTransactionParams,
_context: RequestContext = RequestContext.API,
): Promise {
// Default parameters
const defaultParams = {
amountReference: 'from' as const,
isAmountInDecimals: false,
};

let apiParams = getAPIParamsForToken({
...defaultParams,
...params,
});
if ('error' in apiParams) {
return apiParams;
}

if (params.useAggregator && params.amountReference === 'to') {
console.error(SwapMessage.UNSUPPORTED_AMOUNT_REFERENCE);
return {
code: UNSUPPORTED_AMOUNT_REFERENCE_ERROR_CODE,
error: SwapMessage.UNSUPPORTED_AMOUNT_REFERENCE,
message: '',
};
}

if (!params.useAggregator) {
apiParams = {
v2Enabled: true,
...apiParams,
};
}
if (params.maxSlippage) {
let slippagePercentage = params.maxSlippage;
// Adjust slippage for V1 API (aggregator)
// V1 expects slippage in tenths of a percent (e.g., 30 = 3%)
if (params.useAggregator) {
slippagePercentage = (Number(params.maxSlippage) * 10).toString();
}
apiParams = {
slippagePercentage,
...apiParams,
};
}

try {
const res = await sendRequest(
CDP_GET_SWAP_TRADE,
[apiParams],
_context,
);
if (res.error) {
return {
code: getSwapErrorCode('swap', res.error?.code),
error: res.error.message,
message: '',
};
}

const trade = res.result;
return {
approveTransaction: trade.approveTx
? getSwapTransaction(trade.approveTx, trade.chainId)
: undefined,
fee: trade.fee,
quote: trade.quote,
transaction: getSwapTransaction(trade.tx, trade.chainId),
warning: trade.quote.warning,
};
} catch {
return {
code: getSwapErrorCode('uncaught-swap'),
error: 'Something went wrong',
message: '',
};
}
}
```

In this case, developers need to parse response manually (eg. `instances of`). Otherwise, linter will report error when you try to use response directly, since the response type might be `APIError`

Image

I think that SDK function shouldn't implement error handling, A better way would be to throw error to end developers, allowing them to decide whether or not to handle the error thrown from SDK functions. Another benefit is that we would have clearer type definition in SDK

I'd love to try to disscuss or solve this issue. If you have more feedback or advice, please let me know!

### Describe alternatives you've considered.

_No response_

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.