dataworld: improve error message #394
Conversation
| @@ -88,6 +88,11 @@ export function query(queryString, connection) { | |||
| body: params | |||
| }) | |||
| .then(res => { | |||
| if (res.status !== 200) { | |||
| return res.text().then(body => { | |||
There was a problem hiding this comment.
Unfortunately not all error messages are returned as text, some come in the form of an error object.
For example, 401s produce the following response:
{ "code": 401, "request": "06d583b7-afbf-48ca-b1b8-961ed0b092a0", "message": "Unauthorized" }
Displaying the entire object is unnecessary since only the content of message is useful to the user.
We could replace this bit of code with:
if (res.status !== 200) {
return res.text().then(body => {
try {
const errorObject = JSON.parse(body);
throw new Error(errorObject.message);
} catch (err) {
if (err.name === 'SyntaxError') {
throw new Error(body);
}
throw new Error(err.message);
}
});
}
There was a problem hiding this comment.
I've handled this in Preview.react.js so that other connectors benefit from this PR.
There was a problem hiding this comment.
Agreed, the changes in Preview.react.js allow all connectors to display a custom error message instead of simply "stringifying" the error thrown.
The change I'm suggesting has to do with the custom error that the data.world connector throws.
When the response from the API is a string it will be displayed as is, e.g.
When it is a 400 the response from the API is a string specifying the specific problem (e.g. '(' expected but 'BADSELECT' found... )
But when the response is an object the entire object is shown, e.g.
When it is a 401 the response from the API is an object that is overly verbose ({ "code": 401, "request": "06d583b7-afbf-48ca-b1b8-961ed0b092a0", "message": "Unauthorized" })
As it currently stands this entire object will be displayed to the user, the change I'm proposing will result in only the contents of the message attribute being displayed to the user i.e. Unauthorized
There was a problem hiding this comment.
@n-riesco no worries!
Yes it does! More elegant than my suggested fix 
| let successMsg = ''; | ||
|
|
||
| let errorMsg = ''; | ||
| function setErrorMsg(error) { | ||
| if (error && error.content && error.content.error && error.content.error.message) { |
There was a problem hiding this comment.
To be more succinct we could go with:
try {
errorMsg = error.content.error.message;
} catch(err) {
errorMsg = JSON.stringify(error);
} finally {
errorMsg = String(errorMsg).trim();
}
* data.world: send error message of failed queries. * Preview: repect line breaks in error messages and use mono-space font. Closes plotly#389
n-riesco
force-pushed
the
dataworld/improve-error-message
branch
from
18b120e to
4b203a2
Compare
|
@kndungu I've updated the PR using your comments and rebased it against master. I'll merge it, if you're happy with the latest changes. |
@kndungu @rflprr Since you are familiar with the code, would you like to review this PR?
data.world: send error message of failed queries.
Preview: repect line breaks in error messages and use mono-space font.
Closes #389