-
Notifications
You must be signed in to change notification settings - Fork 73
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
remove old v0 errors from js sdk. provide error details to the user #385
Conversation
@@ -528,7 +528,7 @@ async function buildFunc() { | |||
.map((item) => item.replace('apps/', '')) | |||
.join(','); | |||
|
|||
const command = `yarn nx run-many --target=build --exclude=${ignoreList}`; | |||
const command = `yarn nx run-many --target=build --exclude=${ignoreList} --parallel=false`; |
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.
This is not related to this PR. Can we remove?
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.
it's the only way i can get it to build consistently :(
anson's optimise/1
branch also fixes the build for me. so my plan is to remove --parallel=false when his optimise/1
branch lands. if we merge optimise/1
first then we could remove --parallel=false now.
* init patch * Feature/lit 2570 js sdk fix build (#387) * feat: make capacityTokenId optional * chore: add test * fix: Conditionally include nft_id * fix: replace nx executor to @nrwl/js:tsc * fix: replace nx executor * feat: migrated nx to 15.9.0 * feat: migration nx from 15.9.0 -> 16.10.0 * feat: migrate nx to 17.3.0 * fix: yarn:test:unit command * chore: log cc nft token info * fix: undefined typings in package.json * remove old v0 errors from js sdk. provide error details to the user (#385) * remove old v0 errors from js sdk. provide error details to the user * building now seems to consistently work * refactored error handling to make it cleaner * Remove unused function * okay also added requestId in a few places * ran the linter * fix: replace nx executor * feat: add gitignore --------- Co-authored-by: Ansonhkg <ansonox@gmail.com> * feat: cache enabled * chore: update .gitignore * chore: enable parallel * feat: make capacityTokenId optional (#384) * feat: make capacityTokenId optional * chore: add test * fix: Conditionally include nft_id * Update yarn.lock to match package.json changes * fix: ReferenceError: crypto is not defined * fix: ignore nx cache directory on linting * fix: ignore tools directory on linting * fix: linting * fix: ' ReferenceError: require is not defined in ES module scope, you can use import instead' * fix: lint * [Release] 3.2.2 * chore: move to manual tests --------- Co-authored-by: Chris Cassano <1285652+glitch003@users.noreply.github.com>
Users often get the error "There was an error getting the signing shares from the nodes". We should aim to include any details we have from the node, and not just return this opaque error. IMO the "After" message below is a lot better - it clearly indicates that there was a timeout.
We can still improve this a lot, but this is just a quick fix to ensure that the user can at least see what's coming out of the node. In reality, this timeout error should be a specific error code, etc, but that might be something we need to fix on the node side (aka, the node isn't returning a proper error code in this case, and it's returning some unknown error, but at least that error has details that help with debugging)
Before:
After:
Or, in the case where we just didn't get enough shares: