-
Notifications
You must be signed in to change notification settings - Fork 404
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
Replace deprecated requests
with node-fetch
#191
Changes from 11 commits
5cee15c
bec747d
bd98f7f
f90d893
eca8edc
a5f960c
0bb0460
2bca72d
8b92079
99bb192
a24d8ec
4672267
d1f8de1
ce08308
f84c989
501310a
32152b6
dbe0992
9b52c14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ To install airtable.js in a node project: | |
|
||
npm install airtable | ||
|
||
Airtable.js should work with Node 10 and above. | ||
|
||
### Browser | ||
|
||
|
@@ -33,6 +34,8 @@ Edit `test/test_files/index.html` - put your `BASE_ID` and `API_KEY` (Be careful | |
|
||
Then open http://localhost:8000/ in your browser. | ||
|
||
Airtable.js should work with browsers supported by Airtable App. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed as well. |
||
See the [techincal requirements](https://support.airtable.com/hc/en-us/articles/217990018) for more details. | ||
|
||
# Configuration | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// istanbul ignore file | ||
if (typeof window === 'undefined') { | ||
module.exports = require('abort-controller'); | ||
} else { | ||
if ('signal' in new Request('')) { | ||
module.exports = window.AbortController; | ||
} else { | ||
var polyfill = require('abortcontroller-polyfill/dist/cjs-ponyfill'); | ||
module.exports = polyfill.AbortController; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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('./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'); | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. im assuming the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was. |
||
} | ||
|
||
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, | ||
}); | ||
}); | ||
}); | ||
}; | ||
|
||
|
@@ -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); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
var fetch = require('node-fetch'); | ||
|
||
// istanbul ignore next | ||
module.exports = typeof window === 'undefined' ? fetch : window.fetch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe
is compatible with
instead ofshould work
? So the wording is a bit more confidence inspiring 😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call.