I am writing a function that serves as an API intermediary function between an actual server API call, and userland. This function validates some input data (sync), transforms it as appropriate(sync), reads some other data (async). The results are then merged and used to finally make the actual call to a server endpoint (async). This function must always return a promise, and all execution flow in higher-level APIs using it should be handled through promise chaining.
However, I am having trouble figuring out if I'm doing it the proper way. And also, I have some related questions that I cannot find an answer to.
This is the code I wrote up until now:
import axios from "axios";
import * as MyAPI from "./MyAPI";
import querystring from "querystring";
function fetchFromServerEndpoint(route, routeParams, optQueryParams) {
// Is it ok to use Promise.all() to combine sync and async code as below?
return Promise.all([
new Promise((resolve, reject) => {
const routeReplacedParams = route.replace(/{([^{/]*?)}/g, (match, paramName) => {
const paramValue = routeParams[paramName];
if (paramValue === undefined) {
// Here I need to terminate execution of the replacer function
// and reject this promise
// Is it ok to `throw` here?
throw "UNDEFINED_ROUTE_PARAMETER";
// Can I somehow `return reject(...)` here instead of throw-ing?
}
return paramValue;
});
return resolve(routeReplacedParams);
}),
// Is it fine to use async to mock a promise with synchronous code?
async function(){
return querystring.stringify(optQueryParams);
}),
// This is part of a custom API, returns a legit promise
MyAPI.getAuthorizationAsync(),
]).then(([finalRoute, queryParams, authToken]) => {
// axios library, all APIs will return a promise
return axios.get(`${finalRoute}?${queryParams}`, {
headers: {
Authorization: `Basic ${authToken}`
}
});
});
}
My questions are:
String.replace, is it ok to 'throw' there? Can you suggest any better option, or maybe, a more promise-friendly approach? async will implictly create a promise, and will also implictly convert (uncaught) throws into Promise.reject()s and returns intro Promise.resolve()s. The benefit is that you can write code identically to a normal synchronous function, but still have it behave like a promise, so you can chain promises to it. Is this understanding of mine incorrect? Are there drawbacks (or benefits, other than what I just described) that I should be aware of? What is a proper way to stop execution of a block of code when an error situation arises, from a syncrhonous function, inside a promise? Moreover, like in the replacer function of String.replace, is it ok to 'throw' there? Can you suggest any better option, or maybe, a more promise-friendly approach?
It is ok to throw there. The Promise executor function does catch synchronous exceptions that occur in the executor function and turn them into a rejected promise.
Whether that's a good thing to do really depends upon what you want to happen. If you want the promise in the calling function to reject, then you can just throw from the synchronous function and that will bubble up to the promise in the asynchronous caller and cause a reject. Otherwise, you're also free to use regular synchronous techniques such as return an error condition from the synchronous function, check for that error in the caller and then act accordingly. So, like any other design, it totally depends upon how you want your synchronous function to be called and how the caller should check for errors or handle errors. Sometimes an error code returned is right and sometimes throwing an exception is right.
As an example, in this if calling randomSyncFunctionThatMightThrow() throws, then the callers promise will get rejected automatically and the .catch() handler will be hit.
function randomSyncFunctionThatMightThrow() {
// other logic
throw new Error("random error");
}
someFuncThatReturnsPromise().then(arg => {
// bunch of logic here
randomSyncFunctionThatMightThrow();
// other logic here
return someThing;
}).then(result => {
console.log(finalResult);
}).catch(err => {
console.log(err);
});
Marking a function with async will implictly create a promise, and will also implictly convert (uncaught) throws into Promise.reject()s and returns intro Promise.resolve()s. The benefit is that you can write code identically to a normal synchronous function, but still have it behave like a promise, so you can chain promises to it. Is this understanding of mine incorrect?
Sounds right to me. async functions have a try/catch build in and automatically catch any exceptions and turn them into a rejected promise.
Are there drawbacks (or benefits, other than what I just described) that I should be aware of?
Forcing all synchronous code to handle normal ordinary return results asynchronously with promises has its limits. You wouldn't use it for every synchronous function in your whole program for obvious coding complexity reasons. Asynchronous code is more time consuming to write and test so I don't go looking for ways to make ordinary synchronous things start returning promises and make all callers deal with the asynchronously. There might be an occasional time when some synchronous code is easier to mix with asynchronous code if it also deals in promises. But, that's not the ordinary use of synchronous code.
I used Promise.all() in order to combine the results of multiple, independent stages of the function execution. I suppose this is fine, but are there any other ways to deal with this situation? Or maybe it's a bad practice? Should I, instead, chain promises, and pass-through results from a promise to the next until, I reach the level where I need to use all of them?
Promise.all() is for situations where you want to have multiple independent and parallel, asynchronous executions chains all in-flight at the same time and you just want to know when they are all done and have all the results at once. Promise chaining is for when you want to sequence things in order one after the other for any number of reasons, but often because step #2 depends upon the result of step #1. You should pick Promise.all() vs. chaining depending upon the needs of the asynchronous code (executing in parallel vs. executing in sequence).
Putting synchronous code into a Promise.all() can work just fine (you don't even have to wrap it in a promise as Promise.all() will accept a return value from running a function too), but usually, there's no point. The synchronous code is blocking and synchronous so you usually don't get any parallelism by putting the synchronous code in the Promise.all(). I would typically just run the synchronous code either before or after the Promise.all() and not mix it in with the actual asynchronous promises.
However, I am having trouble figuring out if I'm doing it the proper way.
I've included below an alternate implementation. I don't, as a general practice, like wrapping synchronous code in promises, so this is my alternative. I've used an async function both so I can use await and so that synchronous exceptions are automatically converted into rejected promise and your function's contract was already returning a promise so this is just a simpler way of achieving the same result (in my opinion). Of course, this is all just coding style which is largely governed by opinions.
Here's how I would likely write your code:
async function fetchFromServerEndpoint(route, routeParams, optQueryParams) {
const finalRoute = route.replace(/{([^{/]*?)}/g, (match, paramName) => {
const paramValue = routeParams[paramName];
if (paramValue === undefined) {
// Here I need to abort execution of the replacer function
// parent promise will be rejected
throw new Error("UNDEFINED_ROUTE_PARAMETER");
}
return paramValue;
});
const queryParms = querystring.stringify(optQueryParams);
const authToken = await MyAPI.getAuthorizationAsync();
return axios.get(`${finalRoute}?${queryParams}`, {
headers: {
Authorization: `Basic ${authToken}`
}
});
}
If you really don't like await, you can avoid that here by changing the end to:
return MyAPI.getAuthorizationAsync().then(authToken => {
return axios.get(`${finalRoute}?${queryParams}`, {
headers: {
Authorization: `Basic ${authToken}`
}
});
});
But, as I've said in a comment elsewhere, I think await is a useful tool (when used appropriately) that lets you write simpler code.