-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
chore: remove got dependency #6497
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any changes to yarn.lock
?
The correct way to remove a dependency is to use the yarn remove
command.
packages/cli/test/utils/simulation/beacon_clients/lighthouse.ts
Outdated
Show resolved
Hide resolved
packages/cli/test/utils/simulation/beacon_clients/lighthouse.ts
Outdated
Show resolved
Hide resolved
@jeluard is there a difference between that and just removing it from package.json + running yarn install afterwards? |
Probably not, somehow it feels safer to use |
Co-authored-by: Julien <[email protected]>
Co-authored-by: Julien <[email protected]>
Sorry, should delete by |
Co-authored-by: Nico Flaig <[email protected]>
Co-authored-by: Nico Flaig <[email protected]>
packages/cli/test/utils/simulation/beacon_clients/lighthouse.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure to verify locally that your changes at least build and there are no lint issues
At a minimum run
yarn build
(oryarn check-types
)- and
yarn lint
Remaining task
if (err instanceof FetchError && err.code !== "ECONNREFUSED") {
return {ok: true};
}
4XX-5XX ErrorError: Error fetching bootnodes file: Status Code 404 (Not Found)
at fetchBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:116:11)
at processTicksAndRejections (node:internal/process/task_queues:95:5)
at getNetworkBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:128:24)
at beaconHandlerInit (file:///home/hnaito/lodestar/packages/cli/src/cmds/beacon/handler.ts:179:35) Other errorError fetching latest bootnodes: FetchError: Request to https://example-example-example.com/notfound failed, reason: getaddrinfo ENOTFOUND example-example-example.com
at wrappedFetch (file:///home/hnaito/lodestar/packages/api/src/utils/client/fetch.ts:12:11)
at processTicksAndRejections (node:internal/process/task_queues:95:5)
at fetchBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:114:15)
at getNetworkBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:128:24)
at beaconHandlerInit (file:///home/hnaito/lodestar/packages/cli/src/cmds/beacon/handler.ts:179:35)
at Object.beaconHandler [as handler] (file:///home/hnaito/lodestar/packages/cli/src/cmds/beacon/handler.ts:36:84) @nflaig So far, pointed codes are corrected. However, you mentioned the nested Error still Nesting by FetchError. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeluard I coded in network.ts as a utility function by wrapedFetch.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #6497 +/- ##
============================================
- Coverage 61.72% 61.71% -0.01%
============================================
Files 555 555
Lines 58204 58217 +13
Branches 1843 1844 +1
============================================
+ Hits 35926 35931 +5
- Misses 22239 22247 +8
Partials 39 39 |
@@ -102,13 +101,20 @@ export function getGenesisFileUrl(network: NetworkName): string | null { | |||
* Fetches the latest list of bootnodes for a network | |||
* Bootnodes file is expected to contain bootnode ENR's concatenated by newlines | |||
*/ | |||
export async function fetchBootnodes(network: NetworkName): Promise<string[]> { | |||
|
|||
class FetchBootFileError extends Error {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I move the CustomError to the file?
lodestar/packages/utils/src/errors.ts
Line 13 in b5712a6
export class LodestarError<T extends {code: string}> extends Error { |
@@ -121,8 +127,13 @@ export async function getNetworkBootnodes(network: NetworkName): Promise<string[ | |||
const bootEnrs = await fetchBootnodes(network); | |||
bootnodes.push(...bootEnrs); | |||
} catch (e) { | |||
// eslint-disable-next-line no-console | |||
console.error(`Error fetching latest bootnodes: ${(e as Error).stack}`); | |||
if (e instanceof FetchBootFileError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I added the logic, the nested Error mesage resolved.
if (e instanceof FetchBootFileError) { | |
if (e instanceof FetchBootFileError) { | |
// eslint-disable-next-line no-console | |
console.error(`${(e as Error).stack}`); | |
} | |
if (e instanceof FetchError) { | |
console.error(`${(e as Error).stack}`); | |
} |
The message becomes
FetchError: Request to https://example-example-example.com/notfound failed, reason: getaddrinfo ENOTFOUND example-example-example.com
at wrappedFetch (file:///home/hnaito/lodestar/packages/api/src/utils/client/fetch.ts:12:11)
at processTicksAndRejections (node:internal/process/task_queues:95:5)
at fetchBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:114:15)
at getNetworkBootnodes (file:///home/hnaito/lodestar/packages/cli/src/networks/index.ts:128:24)
at beaconHandlerInit (file:///home/hnaito/lodestar/packages/cli/src/cmds/beacon/handler.ts:179:35)
at Object.beaconHandler [as handler] (file:///home/hnaito/lodestar/packages/cli/src/cmds/beacon/handler.ts:36:84)
closing as stale |
Motivation
For the solution of #6488
Description
Changed all codes which use
got
tofetch
module.Closes # #6488
Steps to test or reproduce