Skip to content
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

core: filter out duplicate error messages #1055

Merged
merged 1 commit into from
Jan 7, 2016
Merged

core: filter out duplicate error messages #1055

merged 1 commit into from
Jan 7, 2016

Conversation

callmehiphop
Copy link
Contributor

In certain cases we are seeing duplicate error messages. This change will filter out any duplicates to avoid error messages like Not Found - Not Found.

@callmehiphop callmehiphop added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. core labels Jan 7, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 7, 2016
@stephenplusplus
Copy link
Contributor

To answer your earlier question, I guess this is why this whole idea was kinda weird. Maybe we should just revert the change and only special-case the error messages for Datastore? That's where we saw the vagueness originally.

@callmehiphop
Copy link
Contributor Author

I think it's still useful, it was really only behaving oddly in Storage, but if you'd rather special-case Datastore, that's cool with me too.

@@ -81,13 +81,19 @@ var ApiError = createErrorClass('ApiError', function(errorBody) {

if (errorBody.errors && errorBody.errors.length === 1) {
messages.push(errorBody.errors[0].message);
} else if (errorBody.response.body) {
} else if (errorBody.response && errorBody.response.body) {

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

I'm cool with keeping the feature if you see it's useful. Let's see what happens!

installed array-uniq as dependency
@stephenplusplus
Copy link
Contributor

Cool. Pretty sure the answer is yes, but did you confirm the system tests are happy now?

@callmehiphop
Copy link
Contributor Author

Sure did!

stephenplusplus added a commit that referenced this pull request Jan 7, 2016
core: filter out duplicate error messages
@stephenplusplus stephenplusplus merged commit dec7234 into googleapis:master Jan 7, 2016
@callmehiphop callmehiphop deleted the error-message-tests branch January 7, 2016 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. core type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants