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

Migrate to ofetch #904

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
1c2f3d9
chore: migrate fetch lib from cross-fetch to ofetch
wa0x6e Sep 20, 2023
0304356
feat: add timeout to fetch requests
wa0x6e Sep 21, 2023
60faabe
chore: fix tests
wa0x6e Sep 21, 2023
bf0c869
Merge branch 'master' into migrate-ofetch
wa0x6e Sep 21, 2023
170b932
chore: add more tests
wa0x6e Sep 21, 2023
b212168
chore: add more test
wa0x6e Sep 21, 2023
a910087
Merge branch 'master' into migrate-ofetch
wa0x6e Sep 22, 2023
3dc29cb
chore: remove uneeded JSON.stringify
wa0x6e Sep 22, 2023
6bc962f
Merge branch 'migrate-ofetch' of https://github.com/snapshot-labs/sna…
wa0x6e Sep 22, 2023
77a6c2f
chore: split long line
wa0x6e Sep 22, 2023
cfd5246
fix: handle errors when parsing json response
wa0x6e Sep 22, 2023
fc18bab
fix: migrate fetch request to ofetch API
wa0x6e Sep 22, 2023
836dddf
chore: bump version
wa0x6e Sep 25, 2023
bc64032
chore: add more todo test
wa0x6e Sep 27, 2023
137ffa1
chore: rename test function
wa0x6e Sep 27, 2023
2cb0b93
chore: use .js extension for test files
wa0x6e Sep 27, 2023
925f2b8
chore: improve/add more tests
wa0x6e Sep 27, 2023
ba1889c
chore: add more tests
wa0x6e Sep 28, 2023
83981d4
Merge branch 'master' into migrate-ofetch
wa0x6e Sep 28, 2023
1610b3d
chore: fix tests
wa0x6e Sep 28, 2023
305935b
chore: fix tests
wa0x6e Sep 28, 2023
bfad8f7
fix: add more tests
wa0x6e Sep 28, 2023
3bb41c9
chore: fix tests
wa0x6e Sep 28, 2023
3d1dc2a
chore: remove redundant function calls
wa0x6e Sep 29, 2023
dbc44e0
chore: fix test
wa0x6e Sep 29, 2023
261a6e4
chore: add retry to flaky tests
wa0x6e Sep 29, 2023
58fc436
chore: improve tests
wa0x6e Sep 29, 2023
520b95e
Merge branch 'master' into migrate-ofetch
wa0x6e Oct 1, 2023
96d7df1
Merge branch 'master' into migrate-ofetch
wa0x6e Oct 2, 2023
93f8069
chore: fix tests for node 18
wa0x6e Oct 3, 2023
bec2e3a
Merge branch 'master' into migrate-ofetch
ChaituVR Oct 5, 2023
b2cc4b2
Merge branch 'master' into migrate-ofetch
wa0x6e Oct 9, 2023
cfb0e3e
chore: bump feat version instead of major
wa0x6e Oct 9, 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 package.json
wa0x6e marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
"@ethersproject/wallet": "^5.6.2",
"ajv": "^8.11.0",
"ajv-formats": "^2.1.1",
"cross-fetch": "^3.1.6",
"json-to-graphql-query": "^2.2.4",
"lodash.set": "^4.3.2"
"lodash.set": "^4.3.2",
"ofetch": "^1.3.3"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^18.1.0",
Expand Down
5 changes: 3 additions & 2 deletions src/sign/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fetch from 'cross-fetch';
import { ofetch as fetch } from 'ofetch';
import { Web3Provider } from '@ethersproject/providers';
import { Wallet } from '@ethersproject/wallet';
import { getAddress } from '@ethersproject/address';
Expand Down Expand Up @@ -87,7 +87,8 @@ export default class Client {
Accept: 'application/json',
'Content-Type': 'application/json'
},
body: JSON.stringify(envelop)
timeout: 20e3,
body: envelop
};
return new Promise((resolve, reject) => {
fetch(address, init)
Expand Down
119 changes: 59 additions & 60 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fetch from 'cross-fetch';
import { ofetch as fetch } from 'ofetch';
import { Interface } from '@ethersproject/abi';
import { Contract } from '@ethersproject/contracts';
import { isAddress } from '@ethersproject/address';
Expand Down Expand Up @@ -136,34 +136,29 @@ export async function multicall(
}

export async function subgraphRequest(url: string, query, options: any = {}) {
const res = await fetch(url, {
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
...options?.headers
},
body: JSON.stringify({ query: jsonToGraphQLQuery({ query }) })
});
let responseData: any = await res.text();
try {
responseData = JSON.parse(responseData);
const init = {
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
...options?.headers
},
timeout: 20e3,
body: { query: jsonToGraphQLQuery({ query }) }
};

const body = await fetch(url, init);
return body.data;
} catch (e) {
throw new Error(
`Errors found in subgraphRequest: URL: ${url}, Status: ${
res.status
}, Response: ${responseData.substring(0, 400)}`
);
}
if (responseData.errors) {
throw new Error(
`Errors found in subgraphRequest: URL: ${url}, Status: ${
res.status
}, Response: ${JSON.stringify(responseData.errors).substring(0, 400)}`
return Promise.reject(
e.data?.error || {
code: e.status || 0,
message: e.statusText || e.toString(),
data: e.data || ''
}
);
}
const { data } = responseData;
return data || {};
}

export function getUrl(uri, gateway = gateways[0]) {
Expand All @@ -186,7 +181,7 @@ export function getUrl(uri, gateway = gateways[0]) {

export async function getJSON(uri, options: any = {}) {
const url = getUrl(uri, options.gateways);
return fetch(url).then((res) => res.json());
return fetch(url, { timeout: 30e3 }).then((res) => res.json());
}

export async function ipfsGet(
Expand All @@ -195,7 +190,7 @@ export async function ipfsGet(
protocolType = 'ipfs'
) {
const url = `https://${gateway}/${protocolType}/${ipfsHash}`;
return fetch(url).then((res) => res.json());
return fetch(url, { timeout: 20e3 }).then((res) => res.json());
}

export async function sendTransaction(
Expand Down Expand Up @@ -234,25 +229,25 @@ export async function getScores(
strategies,
addresses
};
const res = await fetch(scoreApiUrl, {

const body = await fetch(scoreApiUrl, {
method: 'POST',
headers: scoreApiHeaders,
body: JSON.stringify({ params })
timeout: 60e3,
body: { params }
Todmy marked this conversation as resolved.
Show resolved Hide resolved
});
const obj = await res.json();

if (obj.error) {
return Promise.reject(obj.error);
Todmy marked this conversation as resolved.
Show resolved Hide resolved
}

return options.returnValue === 'all'
? obj.result
: obj.result[options.returnValue || 'scores'];
? body.result
: body.result[options.returnValue || 'scores'];
} catch (e) {
if (e.errno) {
return Promise.reject({ code: e.errno, message: e.toString(), data: '' });
}
return Promise.reject(e);
return Promise.reject(
e.data?.error || {
code: e.status || 0,
message: e.statusText || e.toString(),
data: e.data || ''
Todmy marked this conversation as resolved.
Show resolved Hide resolved
}
);
}
}

Expand All @@ -270,7 +265,8 @@ export async function getVp(
const init = {
method: 'POST',
headers: scoreApiHeaders,
body: JSON.stringify({
timeout: 60e3,
body: {
jsonrpc: '2.0',
method: 'get_vp',
params: {
Expand All @@ -281,19 +277,20 @@ export async function getVp(
space,
delegation
}
})
}
};

try {
const res = await fetch(options.url, init);
const json = await res.json();
if (json.error) return Promise.reject(json.error);
if (json.result) return json.result;
const body = await fetch(options.url, init);
Comment on lines -290 to -291
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume sometimes a successful response could look like this:

{
  "error": "some error"
}

Also, consider that the getVp function returned the result if body.result otherwise it returned undefined. It can be an error from the past, but better to check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, no 200 response should ever return an object with the error key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, consider that the getVp function returned the result if body.result otherwise it returned undefined. It can be an error from the past, but better to check

What do you mean by that ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean after this line it returns nothing(undefined). But your implementation always returns data. As I said, it could be an error in the previous version, but better to check in places where we use getVp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're reading the wrong commit, the line you're mentioning have been removed

return body.result;
} catch (e) {
if (e.errno) {
return Promise.reject({ code: e.errno, message: e.toString(), data: '' });
Todmy marked this conversation as resolved.
Show resolved Hide resolved
}
return Promise.reject(e);
return Promise.reject(
e.data?.error || {
code: e.status || 0,
message: e.statusText || e.toString(),
data: e.data || ''
}
);
}
}

Expand All @@ -311,7 +308,8 @@ export async function validate(
const init = {
method: 'POST',
headers: scoreApiHeaders,
body: JSON.stringify({
timeout: 30e3,
body: {
jsonrpc: '2.0',
method: 'validate',
params: {
Expand All @@ -322,19 +320,20 @@ export async function validate(
snapshot,
params
}
})
}
};

try {
const res = await fetch(options.url, init);
const json = await res.json();
if (json.error) return Promise.reject(json.error);
return json.result;
const body = await fetch(options.url, init);
return body.result;
} catch (e) {
if (e.errno) {
return Promise.reject({ code: e.errno, message: e.toString(), data: '' });
}
return Promise.reject(e);
return Promise.reject(
e.data?.error || {
code: e.status || 0,
message: e.statusText || e.toString(),
data: e.data || ''
}
);
}
}

Expand Down
Loading
Loading