3

my aim is to fetch data from two URLs and perform an action only when both have come back successfully. On the other hand i want to return an error if either of them fail. I have played around with my code and managed to get the desired effect.

My question is, is there a more efficient, succinct way of achieving the same functionality?

Helper functions

let status = (r) => {  
  if (r.ok) {  
    return Promise.resolve(r)  
  } else {  
    return Promise.reject(new Error(r.statusText))  
  }  
}

let json = (r) => r.json();

Requests

let urls = [
    'http://localhost:3000/incomplete',
    'http://localhost:3000/complete'
]

let promises = urls.map(url => {

    return fetch(url)  
    .then(status)  
    .then(json)  
    .then(d => Promise.resolve(d))
    .catch(e => Promise.reject(new Error(e)));

});

Promise.all(promises).then(d => {
    // do stuff with d
}).catch(e => {
    console.log('Whoops something went wrong!', e);
});
Samuel
  • 2,485
  • 5
  • 30
  • 40
  • 3
    If your code works then https://codereview.stackexchange.com/ might be a better place to ask. At a first glance the `.then(d => Promise.resolve(d))` and `.catch(e => Promise.reject(new Error(e)));` lines seem unnecessary (they don't do anything that wouldn't already happen). Also the `status` function could just be `if (r.ok) { return r; } else {throw new Error(r.statusText); }`. No need to create promises where they are not necessary. – Felix Kling Jun 22 '17 at 07:59
  • Inside your map function don't call "then" and the likes just return fetch(url) – Paul Okeke Jun 22 '17 at 08:00
  • @FelixKling thanks! i had no idea that site existed! Brilliant, i have reduced that line down to `let promises = urls.map(url => fetch(url).then(status).then(json));` – Samuel Jun 22 '17 at 08:12
  • @PaulOkeke If i just return `fetch(url)` in `map` then how can i check my response was okay? – Samuel Jun 22 '17 at 08:13
  • @Samuel you will check each of all the response in Promises.all . e.g Promises.all (promises).then(allfetch => { allfetch [0], allfetch [1].... }) . Promise.all will automatically invoke the "then" function on all the promises and the results of all the promises are returned as an array un the order they were added. Im currently typing with my phone... you can check the docs. https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise/all – Paul Okeke Jun 22 '17 at 16:12
  • @PaulOkeke Calling `.then` inside the map function is superior, since it lets steps run in parallel. – jib Jun 23 '17 at 00:45
  • Does this answer your question? [How can I fetch an array of URLs with Promise.all?](https://stackoverflow.com/questions/31710768/how-can-i-fetch-an-array-of-urls-with-promise-all) – Henke May 20 '21 at 09:56

3 Answers3

5

// https://jsonplaceholder.typicode.com - Provides test JSON data
var urls = [
  'https://jsonplaceholder.typicode.com/todos/1',
  'https://jsonplaceholder.typicode.com/todos/2',
  'https://jsonplaceholder.typicode.com/posts/1',
  'https://jsonplaceholder.typicode.com/posts/2'
];

// Maps each URL into a fetch() Promise
var requests = urls.map(function(url){
  return fetch(url)
  .then(function(response) {
    // throw "uh oh!";  - test a failure
    return response.json();
  })  
});

// Resolve all the promises
Promise.all(requests)
.then((results) => {
  console.log(JSON.stringify(results, null, 2));
}).catch(function(err) {
  console.log("returns just the 1st failure ...");
  console.log(err);
})
<script src="https://getfirebug.com/firebug-lite-debug.js"></script>
Tony O'Hagan
  • 21,638
  • 3
  • 67
  • 78
3
const urls = [
    'http://localhost:3000/incomplete',
    'http://localhost:3000/complete'
]
const json = (r) => r.json()
const status = (r) => r.ok ? Promise.resolve(r) : Promise.reject(new Error(r.statusText))
const toRequest = url => fetch(url).then(status).then(json)
const onError = e => { console.log('Whoops something went wrong!', e) }
const consumeData = data => { console.log('data: ', data) }

Promise.all(urls.map(toRequest))
  .then(consumeData)
  .catch(onError)
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
foundling
  • 1,695
  • 1
  • 16
  • 22
2

Use fetchOk for nicer error messages, and destructuring to access the results:

let fetchOk = (...args) => fetch(...args)
  .then(res => res.ok ? res : res.json().then(data => {
    throw Object.assign(new Error(data.error_message), {name: res.statusText});
  }));

Promise.all([
  'http://localhost:3000/incomplete',
  'http://localhost:3000/complete'
].map(url => fetchOk(url).then(r => r.json()))).then(([d1, d2]) => {
  // do stuff with d1 and d2
}).catch(e => console.error(e));


// Work around stackoverflow's broken error logger.
var console = { error: msg => div.innerHTML += msg + "<br>" };
<div id="div" style="white-space: pre;"></div>
jib
  • 40,579
  • 17
  • 100
  • 158
  • I don't see how that is "a more efficient or succinct way" than what the OP already has. Also the OP doesn't want to call `.json()` in the error case. – Bergi Jun 23 '17 at 05:46
  • @Bergi The error message is in JSON. Don't you want to know what went wrong? - I think most people would prefer a fetch function that throws on not-ok, and throws an informative error. Otherwise we end up having to remember to check status on every fetch, which is prone to mistakes. That's a better re-use pattern, making it more succinct. – jib Jun 23 '17 at 12:14
  • How do you know the error message is in JSON? For all we know, the OP asked for a code with the same functionality that he already has. – Bergi Jun 23 '17 at 13:31
  • @Bergi The OP is expecting json on success, and wants `!r.ok` to throw an error, so I assumed any error message would be in json as well. Is this not common? The OP's example doesn't actually demonstrate this failure path, but this seems to work when I `Run code snippet` in my [other answer](https://stackoverflow.com/a/41517404/918910). – jib Jun 23 '17 at 13:55