-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Include the error message as well as error code in the error message #7762
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
Conversation
| const cause = await this.decodeResponse(response); | ||
|
|
||
| throw new DataConnectError( | ||
| `Request failed with status ${response.status}`, |
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 swallowed the error message here.
| clientResponse as ClientResponse<ExecuteGraphqlResponseError>; | ||
| response as ClientResponse<ExecuteGraphqlResponseError>; | ||
| throw new DataConnectError( | ||
| `Request failed with status ${clientResponse.status}`, |
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 swallowed the error message here.
| const client = new Client({ | ||
| urlPrefix: endpoint, | ||
| apiVersion: "v1beta", | ||
| auth: true, |
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 revert this to use "x-mantle-admin": "all" instead. auth: true may send production auth tokens to the emulator and may run into issues when developing offline.
If the Client API doesn't do what we want, then using fetch is also okay for now. We can always clean code up later but let's not mess with the behavior.
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.
What about remove auth: true here?
x-mantle-admin doesn't matter.
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 tested executing against emulator. Everything works fine, both error and success.
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.
Here are some emulator logs: https://paste.googleplex.com/6332029630414848
Good catch on not sending auth token.
Emulator has V(3) debug logs for the auth token, won't log by default even if someone sends a real token to it.
Tested here. Having the error message makes it so much easy to tell what happened.
We can polish up additional error handling later.
To prod:

To emulator:
