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.

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

feat(client): migrate to json-rpc #81

Merged
merged 5 commits into from
May 20, 2024
Merged

feat(client): migrate to json-rpc #81

merged 5 commits into from
May 20, 2024

Conversation

KwilLuke
Copy link
Contributor

@KwilLuke KwilLuke commented May 9, 2024

The client.ts now uses kwil-db's JSONRPC endpoints (instead of REST).

BREAKING CHANGE: Kwil-JS now relies on kwil-db's JSONRPC endpoints (available in kwil-db v0.8+). From this commit on, you should only use Kwil-JS with kwil-db v0.8+.

Comment on lines 303 to 336
function checkRes<T, R>(
res: GenericResponse<T>,
selector?: (r: T) => R | undefined
res: AxiosResponse<JsonRPCResponse<T>>,
selector: (r: JsonRPCResponse<T>) => R | undefined
): GenericResponse<R> {
if (res.status != 200 || !res.data) {
throw new Error(
JSON.stringify(res.data) ||

switch (res.status) {
case 200:
break;
case 401:
throw new Error(JSON.stringify(res.data) || 'Unauthorized.');
case 404:
throw new Error(JSON.stringify(res.data) || 'Not found.');
case 500:
throw new Error(JSON.stringify(res.data) || 'Internal server error.');
default:
throw new Error(
JSON.stringify(res.data) ||
'An unknown error has occurred. Please check your network connection.'
);
);
}

if (!res.data) {
throw new Error(`failed to parse response: ${res}`);
}

if (res.data.error) {
const data = res.data.error.data ? `, data: ${JSON.stringify(res.data.error.data)}` : '';
throw new Error(`JSON RPC call error: code: ${res.data.error.code}, message: ${res.data.error.message}` + data);
}

if (res.data.jsonrpc !== '2.0') {
throw new Error(JSON.stringify(res.data) || 'Invalid JSON RPC response.');
}

if (!selector) {
return { status: res.status, data: undefined };
if (!res.data.result) {
throw new Error(JSON.stringify(res.data) || 'No result in JSON RPC response.');
}

return {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jchappelow, do you mind taking a look at this checkRes function? I tried to model similar to how you showed me here.

@KwilLuke KwilLuke requested a review from jchappelow May 9, 2024 23:03
Comment on lines 380 to 381
// @ts-ignore - DO NOT MERGE WITHOUT RESOLVING - Jon is looking into why fee is unmarshalled to an integer.
fee: Number(newTx.body.fee),
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure the resolution is simple, but need to make sure this is as expected: kwilteam/kwil-db#713

The client.ts now uses kwil-db's JSONRPC endpoints (instead of REST).

BREAKING CHANGE: Kwil-JS now relies on kwil-db's JSONRPC endpoints (available in kwil-db v0.8+).
From this commit on, you should only use Kwil-JS with kwil-db v0.8+.
change CI to use kwil-db json RPC endpoints
Changed KGW APIs to use JSON RPC.
let msg = '';
msg += `${domain} wants you to sign in with your account:\n`;
msg += `\n`;
if (authParam.statement !== '') {
msg += `${authParam.statement}\n`;
}
msg += '\n';
msg += `URI: ${target.href}\n`;
// @Yaiba: Should I trust the URI provided by KGW or should I use the domain / create my own?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yaiba, can you advise on this? Should I trust the URI provided by KGW or should I create my own?

Copy link

Choose a reason for hiding this comment

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

Since we have checked that the domain matches, KGW will always return domain + /rpc/v1, we can trust returned URI.

@KwilLuke KwilLuke mentioned this pull request May 14, 2024
Fixed a minor bug that cleared all cookies on an individual logout for a multi-user session in
NodeKwil. Now, kwil-js appropriately manages all cookies for different sessions.
Added `EncodableDatabase` type, which is to be used before RLP-encoding a schema. It allows
`rlp:nil` fields to be an empty array.
@Yaiba
Copy link

Yaiba commented May 20, 2024

Should we merge this? I have this branch tested kwilteam/kwil-db#746 in CI https://github.com/kwilteam/kwil-db/actions/runs/9161574685/job/25186730490 and passed

@jchappelow
Copy link
Member

Should we merge this? I have this branch tested kwilteam/kwil-db#746 in CI https://github.com/kwilteam/kwil-db/actions/runs/9161574685/job/25186730490 and passed

Just a sec, scanning through the main handler

@jchappelow
Copy link
Member

Yeah, lgtm. Fire when ready @KwilLuke

@KwilLuke KwilLuke merged commit 2fcf781 into main May 20, 2024
2 checks passed
@KwilLuke KwilLuke deleted the feat/json-rpc branch May 20, 2024 16:46
@KwilLuke KwilLuke linked an issue Jun 7, 2024 that may be closed by this pull request
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.

Problem: foreign_call not supported in kwil-js
3 participants