-
Notifications
You must be signed in to change notification settings - Fork 15
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
Pick nodes consistently in "npm run dev", and query fixes #394
Conversation
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
errorMsg: string, | ||
ignoreStatuses: number[] = [] | ||
) => { | ||
const res = await resFn(); |
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 think this was the key fix - that resFn
wasn't being await'ed in the previous param structure of returnResponse
} catch { | ||
return {}; | ||
} | ||
return (await res.json()).result; |
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.
The original goal of this try catch was to quietly handle cases where the value returned is not a valid JSON. Note that with this change we assume the response will always be a valid JSON.
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.
Very helpful to have that - I couldn't work that out from the code alone, as it seemed to be deliberately translating errors in a success payload. I thought that was the intent of the code.
A note that I didn't see a change / comments / intent described in the changes under #399, so if you have more updates to make a single approach to the different types of errors coming back to the UI true, that would be great.
The "error" categories include:
- Something that isn't JSON/RPC came back - like a
504
gateway error, which won't be JSON parsable - An error specific to a JSON/RPC call came back, with an error code, which could be displayed to a user
- An error specific to a JSON/RPC call came back, which in that particular case the UI wants to hide from the user (like the fact there is no domain receipt on the node)
null
came back from a query describing a straight forward not found scenario
Signed-off-by: Gabriel Indik <[email protected]>
Further UI improvements
Proposed changes
Enhancements to:
npm run dev
thatnode
is on 3000