Some thoughts:
- You are using
async
but not "using" it. You can simplify this for the reader (and often subsequent code) by doingconst accessToken = await axios.post(...);
and similar to de-nest and make the code more understandable. This also helps your avoid bad variable names likeres1
,res2
,res3
, etc. Promise.all()
is your friend when it comes to making batch calls like this. Combined with a tool like yourwaitFor
function, you can introduce any kind of request pattern you want. Right now you are making calls in series which is kind of harder to throttle when it comes to meeting rate limits, as not you are concerned about accounting for response times in determine when to make next call vs. an up-front.- If you are going to use a tool like axios or Fetch API that require a
res.data
orres.json()
type of step, you might consider putting a simple wrapper around the response handling to clean up code a bit. This is also a good way to standardize error handling at the client level. - Break down your code along logical functional lines (another option available to you after using
async
more conventionally). For example, the status request and strapi update are one logical operational unit. - If you control your
strapi
endpoint/database you should consider adding bulk query support for the datapoint updates. Querying in loops is kind of an anti-pattern and pretty much any modern data store supports bulk updates. - I would recommend defining key constants/variables for aiding in code understanding and maintainence. For example, if I wanted to migrate to
v2
of this API at some point, I don't want to have to go replacev1
withv2
in a bunch of places. Centralize how you hold such module "configuration" logic. - Consider implementing more appropriate error handling. My guess is that this server monitoring process shouldn't be designed to just silently swallow errors.
Putting it together might yield something like:
import axios from 'axios';
import strapi from 'your-strapi-module';
const apiBase = `https://api.cloudways.com/api/v1`;
const apiKey = process.env.CLOUDWAYS_API;
const email = process.env.CLOUDWAYS_EMAIL;
const maxRps = 10; // max requests per second for throttling
const throttleInterval = 1000 / maxRps;
// a small wrapper around axios for this particular API
// this might best live in a separate module
const apiRequest = async ({ headers = {}, method = 'get', url }) => {
const opts = {
baseUrl: apiBase,
headers,
method,
url
};
return axios(opts).then( (res) => res.data );
};
// method to obtain OAuth token, again perhaps best in separate module with API axios wrapper.
// this could be made to lazily provision the token on any API request if not already received
const getAccessToken = async () => {
const method = 'post';
const url = `/oauth/access_token?email=${ email }&api_key=${ apiKey }`;
const data = await apiRequest({ method, url});
return data.access_token;
};
const getServerList = async () => strapi.query('server').find({});
const throttle = async (asyncFn, ms) => {
await new Promise( (resolve) => setTimeout(resolve, ms) );
return asyncFn();
};
const updateServerStatus = async (serverId, accessToken) => {
const url = `/server/monitor/summary?server_id=${ serverId }&type=db`;
const headers = {
Authorization: 'Bearer ' + accessToken
};
return apiRequest({ headers, url })
.then( ({ content }) => {
// I am not throttling here as no need to throttle this data store query was noted
// Ideally, this could be made into single update query though
const promises = content.map( (app) => {
return strapi.query('subscription').update(
{ ApplicationSlug: app.name },
{ DiskUsage: Math.ceil(app.datapoint[0]) }
);
});
return Promise.all(promises);
});
};
const updateAll = async () = {
// we can parallelize getting access token and getting server list
const initPromises = [
getServerList(),
getAccessToken()
];
const [ serverList, accessToken ] = await Promise.all(initPromises);
// map your server list to an array of update promises
const updatePromises = serverList.map((server, i) => {
const fn = () => updateServerStatus(server.ServerId, accessToken);
// here is how we stagger the requests to meet rate limits
const wait = i * throttleInterval;
return throttle(fn, wait);
});
return Promise.all(updatePromises);
};
// call the main function somehow (could be exported from module for example)
export {
updateAll as default,
updateAll,
// again, following probably make sense in separate module
apiRequest,
getAccessToken
}
Note that since we are now "using" async
in a more appropriate fashion you now begin to make your logic available in specific functions. For example, if I just wanted to allow one to update the status of a single server, I could expose the updateServerStatus
function in exports. There would have been no way to offer that sort of code flexibility with your current approach of using nested thenables.