Skip to content

Commit

Permalink
treat partial errors as callback(err)ors (#1760)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stephenplusplus @callmehiphop
stephenplusplus authored and callmehiphop committed Nov 7, 2016
1 parent 525fb0f commit b6270e5
Show file tree
Hide file tree
Showing 12 changed files with 364 additions and 231 deletions.
2 changes: 1 addition & 1 deletion packages/bigquery/package.json
View file
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
View file
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
View file
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',
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
View file
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
View file
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
View file
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) {
* // 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
View file
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

0 comments on commit b6270e5

Please sign in to comment.