fix: better error messages for big instructions #2340
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
| if (!connection || !(await isPriceStoreInitialized(connection))) { | ||
| return | ||
| } | ||
| const handleSendProposalButtonClickAsync = async () => { |
There was a problem hiding this comment.
this is just a copy paste refactor so we catch the promise error
There was a problem hiding this comment.
You should remove the async marker from the outer function since it's no longer async.
Also, I generally recommend hoisting this kind of thing to the module scope and passing in arguments. For one, it avoids the perf overhead of creating closures at runtime. For two, it makes the code much easier to follow and understand. For three, explicitly passing arguments rather than relying on closure scoped variables makes the information flow much more obvious.
This function is already gigantic and really hard to trace and understand its flow, breaking it up outside the component would help a lot.
There was a problem hiding this comment.
removed the async keywork, but won't refactor since it's too much work
governance/xc_admin/packages/xc_admin_frontend/components/tabs/General.tsx
Show resolved
Hide resolved
| if (!connection || !(await isPriceStoreInitialized(connection))) { | ||
| return | ||
| } | ||
| const handleSendProposalButtonClickAsync = async () => { |
There was a problem hiding this comment.
You should remove the async marker from the outer function since it's no longer async.
Also, I generally recommend hoisting this kind of thing to the module scope and passing in arguments. For one, it avoids the perf overhead of creating closures at runtime. For two, it makes the code much easier to follow and understand. For three, explicitly passing arguments rather than relying on closure scoped variables makes the information flow much more obvious.
This function is already gigantic and really hard to trace and understand its flow, breaking it up outside the component would help a lot.
| ) => { | ||
| const size = instruction.data.length | ||
| if (size > maxSize) { | ||
| throw new Error( |
There was a problem hiding this comment.
It looks like in this case, you're just using exceptions for control flow. I generally recommend against this pattern, effectively this is a goto pattern in disguise. It's much better to refactor to use actual control flow constructs if it's not too much effort, but I get that this is a big chunk of old code so if it's not worth the effort that's fine too
There was a problem hiding this comment.
not worth the effort
No description provided.