Redundant Promise.resolve / Promise.reject
ID |
javascript.promise_resolve_in_async |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Best Practice |
Language |
JavaScript |
Tags |
best-practice, concurrency, reliability |
Description
Reports Promise.resolve / Promise.reject written where the surrounding construct already settles the promise — returned from an async function, or called as a statement inside a new Promise(…) executor:
async function load(id) {
if (!id) return Promise.reject(new Error("no id")); // FLAW
return Promise.resolve(cache.get(id)); // FLAW
}
new Promise((resolve, reject) => {
if (!ready) { Promise.reject(error); } // FLAW
});Rationale
In an async function. The function already wraps whatever it returns in a resolved promise and whatever it throws in a rejected one. The explicit call therefore puts a promise inside a promise and costs one more microtask turn per call.
The readability cost is larger than the runtime one. return Promise.reject(e) is a rejection written as a return: a try/catch around the call reads as if it could not fire, and a refactor that drops the return — an easy edit, since it looks like an ordinary value — silently turns the rejection into an unhandled one.
In a promise executor. Promise.reject(e) builds a second, unrelated promise and throws it away. The outer promise is never settled, so the caller waits forever, and the discarded rejection surfaces as an unhandled-rejection warning with nothing connecting it to the code that caused it. What was meant is the executor’s own resolve / reject parameter.
Remediation
In an async function, return the value and throw the error:
async function load(id) {
if (!id) throw new Error("no id");
return cache.get(id);
}In an executor, call the parameter the executor was handed:
new Promise((resolve, reject) => {
if (!ready) { reject(error); return; }
resolve(value);
});Notes
Only the two shapes above are reported. The microtask idiom Promise.resolve().then(…) and a Promise.resolve(x) passed on as an argument — Promise.all([Promise.resolve(1), p]) — are left alone: in both, the value is used rather than wrapped and discarded.
A Promise.resolve(…) returned from a plain (non-async) function is the normal way to give a synchronous value a promise interface, and is not reported.