dataworld: improve error message

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.

[n-riesco]

@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

[kndungu]

Thanks for working on this @n-riesco!

Just two minor comments:

[kndungu]

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);
}
});
}

[n-riesco]

I've handled this in Preview.react.js so that other connectors benefit from this PR.

[kndungu]

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

[n-riesco]

@kndungu My bad! Your first explanation was clear enough.

I've fixed this in 11950d1 . I've also tested it manually.

Please, let me know if it looks OK to you.

[kndungu]

@n-riesco no worries!

Yes it does! More elegant than my suggested fix

[kndungu]

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();
}

[n-riesco]

@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.