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

treat partial errors as callback(err)ors #1760

Merged
merged 12 commits into from
Nov 7, 2016
2 changes: 1 addition & 1 deletion packages/bigquery/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"bigquery"
],
"dependencies": {
"@google-cloud/common": "^0.7.0",
"@google-cloud/common": "^0.8.0",
"arrify": "^1.0.0",
"duplexify": "^3.2.0",
"extend": "^3.0.0",
Expand Down
50 changes: 30 additions & 20 deletions packages/bigquery/src/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,7 @@ Table.prototype.import = function(source, metadata, callback) {
* {module:bigquery/table#import} instead.
*
* @resource [Tabledata: insertAll API Documentation]{@link https://cloud.google.com/bigquery/docs/reference/v2/tabledata/insertAll}
* @resource [Troubleshooting Errors]{@link https://developers.google.com/bigquery/troubleshooting-errors}
*
* @param {object|object[]} rows - The rows to insert into the table.
* @param {object=} options - Configuration object.
Expand All @@ -964,7 +965,9 @@ Table.prototype.import = function(source, metadata, callback) {
* for considerations when working with templates tables.
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error returned while making this request.
* @param {array} callback.insertErrors - A list of errors for insert failures.
* @param {object[]} callback.err.errors - If present, these represent partial
* failures. It's possible for part of your request to be completed
* successfully, while the other part was not.
* @param {object} callback.apiResponse - The full API response.
*
* @example
Expand Down Expand Up @@ -1011,22 +1014,22 @@ Table.prototype.import = function(source, metadata, callback) {
* table.insert(row, options, insertHandler);
*
* //-
* // Handling the response.
* // Handling the response. See <a href="https://developers.google.com/bigquery/troubleshooting-errors">
* // Troubleshooting Errors</a> for best practices on how to handle errors.
* //-
* function insertHandler(err, insertErrors, apiResponse) {
* // err (object):
* // An API error occurred.
*
* // insertErrors (object[]):
* // If populated, some rows failed to insert, while others may have
* // succeeded.
* //
* // insertErrors[].row (original individual row object passed to `insert`)
* // insertErrors[].errors[].reason
* // insertErrors[].errors[].message
*
* // See https://developers.google.com/bigquery/troubleshooting-errors for
* // recommendations on handling errors.
* function insertHandler(err, apiResponse) {
* if (err) {
* // An API error or partial failure occurred.
*
* if (err.name === 'PartialFailureError') {
* // Some rows failed to insert, while others may have succeeded.
*
* // err.errors (object[]):
* // err.errors[].row (original row object passed to `insert`)
* // err.errors[].errors[].reason
* // err.errors[].errors[].message
* }
* }
* }
*
* //-
Expand Down Expand Up @@ -1063,23 +1066,30 @@ Table.prototype.insert = function(rows, options, callback) {
json: json
}, function(err, resp) {
if (err) {
callback(err, null, resp);
callback(err, resp);
return;
}

var failedToInsert = (resp.insertErrors || []).map(function(insertError) {
var partialFailures = (resp.insertErrors || []).map(function(insertError) {
return {
errors: insertError.errors.map(function(error) {
return {
message: error.message,
reason: error.reason
};
}),
row: json.rows[insertError.index].json
row: rows[insertError.index]
};
});

callback(null, failedToInsert, resp);
if (partialFailures.length > 0) {
err = new common.util.PartialFailureError({
errors: partialFailures,
response: resp
});
}

callback(err, resp);
});
};

Expand Down
58 changes: 51 additions & 7 deletions packages/bigquery/system-test/bigquery.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ describe('BigQuery', function() {
it('should convert values to their schema types', function(done) {
var data = {
name: 'dave',
breed: 'british shorthair',
breed: 'husky',
id: 99,
dob: new Date(),
around: true,
Expand All @@ -500,14 +500,9 @@ describe('BigQuery', function() {
}
};

table.insert(data, function(err, insertErrors) {
table.insert(data, function(err) {
assert.ifError(err);

if (insertErrors.length > 0) {
done(insertErrors[0].errors[0]);
return;
}

function query(callback) {
var query = {
query: 'SELECT * FROM ' + table.id + ' WHERE id = ' + data.id,
Expand All @@ -533,6 +528,55 @@ describe('BigQuery', function() {
});
});

it('should return partial errors', function(done) {
var data = {
name: 'dave',
breed: 'british husky',

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

id: 99,
dob: new Date(),
around: true,
buffer: new Buffer('test'),
arrayOfInts: [1, 3, 5],
recordOfRecords: {
records: [
{
record: true
}
]
}
};

var improperData = {
name: 11
};

table.insert([data, improperData], function(err) {
assert.strictEqual(err.name, 'PartialFailureError');

assert.deepEqual(err.errors[0], {
errors: [
{
message: 'Conversion from int64 to string is unsupported.',
reason: 'invalid'
}
],
row: improperData
});

assert.deepEqual(err.errors[1], {
errors: [
{
message: undefined,
reason: 'stopped'
}
],
row: data
});

done();
});
});

it('should export data to a file in your bucket', function(done) {
var file = bucket.file('kitten-test-data-backup.json');

Expand Down
24 changes: 14 additions & 10 deletions packages/bigquery/test/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -1221,9 +1221,8 @@ describe('BigQuery/Table', function() {
callback(null, apiResponse);
};

table.insert(data, function(err, insertErrors, apiResponse_) {
table.insert(data, function(err, apiResponse_) {
assert.ifError(err);
assert.deepEqual(insertErrors, []);
assert.strictEqual(apiResponse_, apiResponse);
done();
});
Expand All @@ -1237,15 +1236,14 @@ describe('BigQuery/Table', function() {
callback(error, apiResponse);
};

table.insert(data, function(err, insertErrors, apiResponse_) {
table.insert(data, function(err, apiResponse_) {
assert.strictEqual(err, error);
assert.strictEqual(insertErrors, null);
assert.strictEqual(apiResponse_, apiResponse);
done();
});
});

it('should return insert failures to the callback', function(done) {
it('should return partial failures', function(done) {
var row0Error = { message: 'Error.', reason: 'notFound' };
var row1Error = { message: 'Error.', reason: 'notFound' };

Expand All @@ -1258,12 +1256,18 @@ describe('BigQuery/Table', function() {
});
};

table.insert(data, function(err, insertErrors) {
assert.ifError(err);
table.insert(data, function(err) {
assert.strictEqual(err.name, 'PartialFailureError');

assert.deepEqual(insertErrors, [
{ row: dataApiFormat.rows[0].json, errors: [row0Error] },
{ row: dataApiFormat.rows[1].json, errors: [row1Error] }
assert.deepEqual(err.errors, [
{
row: dataApiFormat.rows[0].json,
errors: [row0Error]
},
{
row: dataApiFormat.rows[1].json,
errors: [row1Error]
}
]);

done();
Expand Down
2 changes: 1 addition & 1 deletion packages/bigtable/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"bigtable"
],
"dependencies": {
"@google-cloud/common": "^0.7.0",
"@google-cloud/common": "^0.8.0",
"arrify": "^1.0.0",
"concat-stream": "^1.5.0",
"create-error-class": "^3.0.2",
Expand Down
74 changes: 31 additions & 43 deletions packages/bigtable/src/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,30 +683,21 @@ Table.prototype.getRows = function(options, callback) {
* See {module:bigtable/table#mutate}.
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error returned while making this request.
* @param {object[]} callback.insertErrors - A status object for each failed
* insert.
* @param {object[]} callback.err.errors - If present, these represent partial
* failures. It's possible for part of your request to be completed
* successfully, while the other part was not.
*
* @example
* var callback = function(err, insertErrors) {
* var callback = function(err) {
* if (err) {
* // Error handling omitted.
* }
* // An API error or partial failure occurred.
*
* // insertErrors = [
* // {
* // code: 500,
* // message: 'Internal Server Error',
* // entry: {
* // key: 'gwashington',
* // data: {
* // follows: {
* // jadams: 1
* // }
* // }
* // }
* // },
* // ...
* // ]
* if (err.name === 'PartialFailureError') {
* // err.errors[].code = 'Response code'
* // err.errors[].message = 'Error message'
* // err.errors[].entry = The original entry
* }
* }
* };
*
* var entries = [
Expand Down Expand Up @@ -769,34 +760,24 @@ Table.prototype.insert = function(entries, callback) {
* deleted.
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error returned while making this request.
* @param {object[]} callback.mutationErrors - A status object for each failed
* mutation.
* @param {object[]} callback.err.errors - If present, these represent partial
* failures. It's possible for part of your request to be completed
* successfully, while the other part was not.
*
* @example
* //-
* // Insert entities. See {module:bigtable/table#insert}
* //-
* var callback = function(err, mutationErrors) {
* var callback = function(err) {
* if (err) {

This comment was marked as spam.

This comment was marked as spam.

* // Error handling omitted.
* }
* // An API error or partial failure occurred.
*
* // mutationErrors = [
* // {
* // code: 500,
* // message: 'Internal Server Error',
* // entry: {
* // method: 'insert',
* // key: 'gwashington',
* // data: {
* // follows: {
* // jadams: 1
* // }
* // }
* // }
* // },
* // ...
* // ]
* if (err.name === 'PartialFailureError') {
* // err.errors[].code = 'Response code'
* // err.errors[].message = 'Error message'
* // err.errors[].entry = The original entry
* }
* }
* };
*
* var entries = [
Expand Down Expand Up @@ -910,7 +891,6 @@ Table.prototype.mutate = function(entries, callback) {
var status = common.GrpcService.decorateStatus_(entry.status);
status.entry = entries[entry.index];


if (!isCallbackMode) {
emitter.emit('error', status);
return;
Expand All @@ -932,7 +912,15 @@ Table.prototype.mutate = function(entries, callback) {
stream
.on('error', callback)
.pipe(concat(function(mutationErrors) {
callback(null, mutationErrors);
var err = null;

if (mutationErrors && mutationErrors.length > 0) {
err = new common.util.PartialFailureError({
errors: mutationErrors
});
}

callback(err);
}));
};

Expand Down
6 changes: 1 addition & 5 deletions packages/bigtable/system-test/bigtable.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,7 @@ describe('Bigtable', function() {
}
}];

TABLE.insert(rows, function(err, insertErrors) {
assert.ifError(err);
assert.strictEqual(insertErrors.length, 0);
done();
});
TABLE.insert(rows, done);
});

it('should create an individual row', function(done) {
Expand Down
Loading