0

I'm using Promise.race so I can time out the fetch call. During a test to an API endpoint that doesn't exist, I'm getting an "Unhandled promise rejection" error. The code is supposed to console.log once upon any failure. Where am I not handling the promise rejection? FYI the main function is function Retrieve

window.path = "http://localhost:3000/records";

function isPrimary(color)  {

    let colorIsPrimary = false;
    (color.startsWith("red") || color.startsWith("blue") || color.startsWith("yellow")) ? colorIsPrimary = true : null;
    return colorIsPrimary;

}

function transformData(records,page) {

    const transformedData = {
        "ids" : [],
        "open": [],
        "closedPrimaryCount": 0,
        "previousPage": null,
        "nextPage": null
    }

    let closedPrimaryCount = 0;

    records.forEach((record, index) => {

        if (index < 10) {

            transformedData.ids.push(record.id);

            record["isPrimary"] = false;
            isPrimary(record.color) ? record.isPrimary = true : null;
        
            record.disposition == "open" ? transformedData.open.push(record) : null;

            if (record.disposition == "closed") {
                isPrimary(record.color) ? closedPrimaryCount++ : null;   
            }

        }

    })

    transformedData.closedPrimaryCount = closedPrimaryCount;

    let previousPage = null;
    page > 1 ? previousPage = page - 1 : null;
    transformedData.previousPage = previousPage;

    let nextPage = null;
    records.length > 10 ? nextPage = page + 1 : null;
    transformedData.nextPage = nextPage;

    return transformedData;
   
}

function promiseTimeout(promise, ms) {

    let timeout = new Promise((resolve, reject) => {
        let timeoutID = setTimeout(() => {
            clearTimeout(timeoutID);
            reject("fetch failed - timeout");
        }, ms)
    })

    return Promise.race([promise, timeout]);

}

function doFetch(url) {

    return new Promise((resolve, reject) => {

        fetch(url)

        .then((response) => {

            if (!response.ok) {
      
                reject(new Error("fetch failed - non 200"));
      
            }

            response.json()
        
            .then((records) => {

                resolve(records);

            })

            .catch((error) => {

                reject(new Error("fetch failed - error from response.json"));
            
            })    

        })

        .catch((error) => {

            reject(new Error("fetch failed - error from fetch"));
        
        })

    }) 

}

function retrieve({page = 1, colors = ["red", "brown", "blue", "yellow", "green"], thetest = false, windowPath = window.path} = {}) { 

    return new Promise((resolve,reject)=>{

        !thetest ? windowPath = "http://localhost:3000/records" : null;

        // limit set to 11 so we can determine nextPage
        const limit = "11";

        const offset = "" + ((page * 10) - 10);

        let colorArgs = "";
        colors.forEach((color, index) => {
            colorArgs = colorArgs + "&color[]=" + color; 
        });

        const requestQuery = `limit=${limit}&offset=${offset}${colorArgs}`;
        
        const requestURL = new URI(windowPath);
        requestURL.query(requestQuery);

        const promiseRace = promiseTimeout(doFetch(requestURL.toString()), 4000);

        promiseRace.then((records) => {

            const transformedData = transformData(records, page);

            resolve(transformedData);

        })

        promiseRace.catch((error) => {

            console.log(error);

        })        

    });

};

export default retrieve;
Ted Fitzpatrick
  • 910
  • 1
  • 8
  • 18
  • 1
    [What is the explicit promise construction antipattern and how do I avoid it?](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it). `fetch` already returns a promise, so `new Promise` should be removed throughout, for starters. See the second answer in this link. Keep in mind also that `reject` doesn't stop execution, so you're still calling `res.json()` on a bad response, and other things like that. I would `throw` if `!response.ok` after making the all-`fetch` refactor. – ggorlen Jun 25 '22 at 18:29
  • 1
    Wow, avoiding that anti-pattern and just using async await is so much easier. I ended up returning an async function. No explicit promise constructors, much more concise and easier to grok. – Ted Fitzpatrick Jun 25 '22 at 19:37
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) in `doFetch`! And in `retrieve` it actually causes the problem, because you're not handling errors on the `promiseRace.then(…);` chain. – Bergi Jun 25 '22 at 20:38
  • Thanks Bergi. Yeah after reading about the anti-pattern I got an "aha" :-) – Ted Fitzpatrick Jun 26 '22 at 03:49

1 Answers1

0

After ggorlen's excellent advice, I refactored to this much cleaner (and test-passing) code:

async function getTransformedData(url,page) {

    try {

        const response = await fetch(url);

        if (response.ok) {

            const records = await response.json();

            const transformedData = transformData(records,page);

            return(transformedData);

        } else {

            throw new Error("failed");    

        }

    }

    catch(error) {

        console.log(error);

    }
    
    // getTransformedData
}


function retrieve({page = 1, colors = ["red", "brown", "blue", "yellow", "green"]} = {}) { 

    // limit set to 11 so we can determine nextPage
    const limit = "11";

    const offset = "" + ((page * 10) - 10);

    let colorArgs = "";
    colors.forEach((color, index) => {
        colorArgs = colorArgs + "&color[]=" + color; 
    });

    const requestQuery = `limit=${limit}&offset=${offset}${colorArgs}`;
    
    const requestURL = new URI(window.path);
    requestURL.query(requestQuery);

    return getTransformedData(requestURL.toString(),page);

};
Ted Fitzpatrick
  • 910
  • 1
  • 8
  • 18