Skip to content

Commit

Permalink
Replace second use of requests module
Browse files Browse the repository at this point in the history
The deprecated requests module was also being used in
`base#makeRequests` even though it was no longer in the list of
installed packages in package.json browserify was pulling it it which
was the root cause of the package being so bloated (because I was
not longer replaceing requests with xhr).

This replaces `request` with `node-fetch` in `makeRequest`.
`makeRequest`s isn't not actually used anywhere. All API calls actually
are going through `runAction` instead and I think maybe `makeRequest`s
should just be removed but in order to do that we need to get more
clarity around what is public vs private.
  • Loading branch information
rmeritz committed Jun 24, 2020
1 parent 0bb0460 commit 2bca72d
Show file tree
Hide file tree
Showing 4 changed files with 1,577 additions and 1,938 deletions.
108 changes: 63 additions & 45 deletions lib/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ var forEach = require('lodash/forEach');
var get = require('lodash/get');
var assign = require('lodash/assign');
var isPlainObject = require('lodash/isPlainObject');
var fetch = require('node-fetch');
var AbortController = require('abort-controller');

// This will become require('xhr') in the browser.
var request = require('request');

var objectToQueryParamString = require('./object_to_query_param_string');
var AirtableError = require('./airtable_error');
var Table = require('./table');
var HttpHeaders = require('./http_headers');
Expand All @@ -34,59 +34,76 @@ Base.prototype.makeRequest = function(options) {

var method = get(options, 'method', 'GET').toUpperCase();

var url =
this._airtable._endpointUrl +
'/v' +
this._airtable._apiVersionMajor +
'/' +
this._id +
get(options, 'path', '/') +
'?' +
objectToQueryParamString(get(options, 'qs', {}));

var controller = new AbortController();

var requestOptions = {
method: method,
url:
this._airtable._endpointUrl +
'/v' +
this._airtable._apiVersionMajor +
'/' +
this._id +
get(options, 'path', '/'),
qs: get(options, 'qs', {}),
headers: this._getRequestHeaders(get(options, 'headers', {})),
json: true,
timeout: this._airtable.requestTimeout,
signal: controller.signal,
};

if ('body' in options && _canRequestMethodIncludeBody(method)) {
requestOptions.body = options.body;
requestOptions.body = JSON.stringify(options.body);
}

var timeout = setTimeout(function() {
controller.abort();
}, this._airtable.requestTimeout);

return new Promise(function(resolve, reject) {
request(requestOptions, function(err, response, body) {
if (!err && response.statusCode === 429 && !that._airtable._noRetryIfRateLimited) {
var numAttempts = get(options, '_numAttempts', 0);
var backoffDelayMs = exponentialBackoffWithJitter(numAttempts);
setTimeout(function() {
var newOptions = assign({}, options, {
_numAttempts: numAttempts + 1,
});
that.makeRequest(newOptions)
.then(resolve)
.catch(reject);
}, backoffDelayMs);
return;
}

if (err) {
fetch(url, requestOptions)
.then(function(resp) {
clearTimeout(timeout);
resp.statusCode = resp.status;
if (resp.status === 429 && !that._airtable._noRetryIfRateLimited) {
var numAttempts = get(options, '_numAttempts', 0);
var backoffDelayMs = exponentialBackoffWithJitter(numAttempts);
setTimeout(function() {
var newOptions = assign({}, options, {
_numAttempts: numAttempts + 1,
});
that.makeRequest(newOptions)
.then(resolve)
.catch(reject);
}, backoffDelayMs);
} else {
resp.json()
.then(function(body) {
var err =
that._checkStatusForError(resp.status, body) ||
_getErrorForNonObjectBody(resp.status, body);

if (err) {
reject(err);
} else {
resolve({
statusCode: resp.status,
headers: resp.headers,
body: body,
});
}
})
.catch(function() {
var err = _getErrorForNonObjectBody(resp.status);
reject(err);
});
}
})
.catch(function(err) {
clearTimeout(timeout);
err = new AirtableError('CONNECTION_ERROR', err.message, null);
} else {
err =
that._checkStatusForError(response.statusCode, body) ||
_getErrorForNonObjectBody(response.statusCode, body);
}

if (err) {
reject(err);
return;
}

resolve({
statusCode: response.statusCode,
headers: response.headers,
body: body,
});
});
});
};

Expand All @@ -100,6 +117,7 @@ Base.prototype._getRequestHeaders = function(headers) {

result.set('Authorization', 'Bearer ' + this._airtable._apiKey);
result.set('User-Agent', userAgent);
result.set('Content-Type', 'application/json');
forEach(headers, function(headerValue, headerKey) {
result.set(headerKey, headerValue);
});
Expand Down
Loading

0 comments on commit 2bca72d

Please sign in to comment.