Skip to main content
forgot a catch
Source Link
elclanrs
  • 2.5k
  • 16
  • 16

A few comments:

  1. fetch already returns a promise, which means this:

    new Promise((resolve, reject) => { return fetch(url).then(response => { if (response.ok) { resolve(response) } else { reject(new Error('error')) } }, error => { reject(new Error(error.message)) }) })

Is pretty much the same as:

fetch(url).then(response => {
  if (response.ok) {
    return response
  }
  return Promise.reject(Error('error'))
}).catch(error => {
  return Promise.reject(Error(error.message))
})
  1. Keeping this in mind you can simplify your code with early returns and fewer branches:

    function get(url) { return fetch(url).then(response => { if (response.ok) { const contentType = response.headers.get('Content-Type') || '';

       if (contentType.includes('application/json')) {
         return response.json().catch(error => {
           return Promise.reject(new ResponseError('Invalid JSON: ' + error.message));
         });
       }
    
       if (contentType.includes('text/html')) {
         return response.text().then(html => {
           return {
             page_type: 'generic',
             html: html
           };
         }).catch(error => {
           return Promise.reject(new ResponseError('HTML error: ' + error.message));
         })
       }
    
       return Promise.reject(new ResponseError('Invalid content type: ' + contentType));
     }
    
     if (response.status == 404) {
       return Promise.reject(new NotFoundError('Page not found: ' + url));
     }
    
     return Promise.reject(new HttpError('HTTP error: ' + response.status));
    

    }).catch(error => { return Promise.reject(new NetworkError(error.message)); }); }

A few comments:

  1. fetch already returns a promise, which means this:

    new Promise((resolve, reject) => { return fetch(url).then(response => { if (response.ok) { resolve(response) } else { reject(new Error('error')) } }, error => { reject(new Error(error.message)) }) })

Is pretty much the same as:

fetch(url).then(response => {
  if (response.ok) {
    return response
  }
  return Promise.reject(Error('error'))
}).catch(error => {
  return Promise.reject(Error(error.message))
})
  1. Keeping this in mind you can simplify your code with early returns and fewer branches:

    function get(url) { return fetch(url).then(response => { if (response.ok) { const contentType = response.headers.get('Content-Type') || '';

       if (contentType.includes('application/json')) {
         return response.json().catch(error => {
           return Promise.reject(new ResponseError('Invalid JSON: ' + error.message));
         });
       }
    
       if (contentType.includes('text/html')) {
         return response.text().then(html => {
           return {
             page_type: 'generic',
             html: html
           };
         }).catch(error => {
           return Promise.reject(new ResponseError('HTML error: ' + error.message));
         })
       }
    
       return Promise.reject(new ResponseError('Invalid content type: ' + contentType));
     }
    
     if (response.status == 404) {
       return Promise.reject(new NotFoundError('Page not found: ' + url));
     }
    
     return Promise.reject(new HttpError('HTTP error: ' + response.status));
    

    }); }

A few comments:

  1. fetch already returns a promise, which means this:

    new Promise((resolve, reject) => { return fetch(url).then(response => { if (response.ok) { resolve(response) } else { reject(new Error('error')) } }, error => { reject(new Error(error.message)) }) })

Is pretty much the same as:

fetch(url).then(response => {
  if (response.ok) {
    return response
  }
  return Promise.reject(Error('error'))
}).catch(error => {
  return Promise.reject(Error(error.message))
})
  1. Keeping this in mind you can simplify your code with early returns and fewer branches:

    function get(url) { return fetch(url).then(response => { if (response.ok) { const contentType = response.headers.get('Content-Type') || '';

       if (contentType.includes('application/json')) {
         return response.json().catch(error => {
           return Promise.reject(new ResponseError('Invalid JSON: ' + error.message));
         });
       }
    
       if (contentType.includes('text/html')) {
         return response.text().then(html => {
           return {
             page_type: 'generic',
             html: html
           };
         }).catch(error => {
           return Promise.reject(new ResponseError('HTML error: ' + error.message));
         })
       }
    
       return Promise.reject(new ResponseError('Invalid content type: ' + contentType));
     }
    
     if (response.status == 404) {
       return Promise.reject(new NotFoundError('Page not found: ' + url));
     }
    
     return Promise.reject(new HttpError('HTTP error: ' + response.status));
    

    }).catch(error => { return Promise.reject(new NetworkError(error.message)); }); }

Source Link
elclanrs
  • 2.5k
  • 16
  • 16

A few comments:

  1. fetch already returns a promise, which means this:

    new Promise((resolve, reject) => { return fetch(url).then(response => { if (response.ok) { resolve(response) } else { reject(new Error('error')) } }, error => { reject(new Error(error.message)) }) })

Is pretty much the same as:

fetch(url).then(response => {
  if (response.ok) {
    return response
  }
  return Promise.reject(Error('error'))
}).catch(error => {
  return Promise.reject(Error(error.message))
})
  1. Keeping this in mind you can simplify your code with early returns and fewer branches:

    function get(url) { return fetch(url).then(response => { if (response.ok) { const contentType = response.headers.get('Content-Type') || '';

       if (contentType.includes('application/json')) {
         return response.json().catch(error => {
           return Promise.reject(new ResponseError('Invalid JSON: ' + error.message));
         });
       }
    
       if (contentType.includes('text/html')) {
         return response.text().then(html => {
           return {
             page_type: 'generic',
             html: html
           };
         }).catch(error => {
           return Promise.reject(new ResponseError('HTML error: ' + error.message));
         })
       }
    
       return Promise.reject(new ResponseError('Invalid content type: ' + contentType));
     }
    
     if (response.status == 404) {
       return Promise.reject(new NotFoundError('Page not found: ' + url));
     }
    
     return Promise.reject(new HttpError('HTTP error: ' + response.status));
    

    }); }