Merge pull request #39 from bugsnag/bengourley/retry

feat: Retry requests in more situations

index.js

@@ -309,10 +309,7 @@ function prepareRequest (options) {
       }
     }
   })
-  return {
-    options,
-    formData
-  }
+  return formData
 }
 
 /**
@@ -323,11 +320,8 @@ function prepareRequest (options) {
  * @param {{options: object, formData: object}}
  * @returns {Promise<string>}
  */
-function sendRequest (args) {
-  // { options, formData }
-  const options = args.options
-  const formData = args.formData
-  return request(options.endpoint, formData).then(() => options)
+function sendRequest (options) {
+  return new Promise((resolve, reject) => request(options.endpoint, () => prepareRequest(options), () => resolve(options), reject))
 }
 
 /**
@@ -372,7 +366,6 @@ function upload (options, callback) {
         return options
       })
       .then(transformOptions)
-      .then(prepareRequest)
       .then(sendRequest)
       .catch(err => {
         return cleanupTempFiles(options)

lib/request.js

@@ -1,32 +1,75 @@
 'use strict'
 
-const request = require('request')
+const https = require('https')
+const http = require('http')
+const concat = require('concat-stream')
+const url = require('url')
+const once = require('once')
+const FormData = require('form-data')
+
 const MAX_ATTEMPTS = 5
-const RETRY_INTERVAL = process.env.BUGSNAG_RETRY_INTERVAL || 1000
+const RETRY_INTERVAL = parseInt(process.env.BUGSNAG_RETRY_INTERVAL) || 1000
+const TIMEOUT = parseInt(process.env.BUGSNAG_TIMEOUT) || 30000
 
-module.exports = (url, data) => {
-  return new Promise((resolve, reject) => {
-    let attempts = 0
-    const maybeRetry = (err) => {
-      attempts++
-      if (err && err.isRetryable && attempts < MAX_ATTEMPTS) return setTimeout(go, RETRY_INTERVAL)
-      return reject(err)
-    }
-    const go = () => send(url, data, resolve, maybeRetry)
-    go()
-  })
+module.exports = (endpoint, makePayload, onSuccess, onError) => {
+  let attempts = 0
+  const maybeRetry = (err) => {
+    attempts++
+    if (err && err.isRetryable !== false && attempts < MAX_ATTEMPTS) return setTimeout(go, RETRY_INTERVAL)
+    return onError(err)
+  }
+  const go = () => send(endpoint, makePayload(), onSuccess, maybeRetry)
+  go()
 }
 
-const send = (url, formData, onSuccess, onError) => {
-  request.post({ url, formData }, (err, res, body) => {
-    if (err || res.statusCode !== 200) {
-      err = err || new Error(`${res.statusMessage} (${res.statusCode}) - ${body}`)
-      if (res && (res.statusCode < 400 || res.statusCode >= 500)) {
-        err.isRetryable = true
+const send = (endpoint, data, onSuccess, onError) => {
+  onError = once(onError)
+  const formData = new FormData()
+  Object.keys(data).forEach(k => formData.append(k, data[k]))
+  const parsedUrl = url.parse(endpoint)
+  const req = (parsedUrl.protocol === 'https:' ? https : http).request({
+    method: 'POST',
+    hostname: parsedUrl.hostname,
+    path: parsedUrl.path || '/',
+    headers: formData.getHeaders(),
+    port: parsedUrl.port || undefined
+  }, res => {
+    res.pipe(concat(body => {
+      if (res.statusCode === 200) return onSuccess()
+      if (res.statusCode !== 400) {
+        const err = new Error(`HTTP status ${res.statusCode} received from upload API`)
+        if (!isRetryable(res.statusCode)) {
+          err.isRetryable = false
+        }
+        return onError(err)
+      }
+      try {
+        const err = new Error('Invalid payload sent to upload API')
+        err.errors = JSON.parse(body.toString()).errors
+        // never retry a 400
+        err.isRetryable = false
+        return onError(err)
+      } catch (_) {
+        const e = new Error(`HTTP status ${res.statusCode} received from upload API`)
+        e.isRetryable = false
+        return onError(e)
       }
-      onError(err)
-    } else {
-      onSuccess()
-    }
+    }))
   })
+  formData.pipe(req)
+  req.on('error', onError)
+  req.setTimeout(TIMEOUT, () => {
+    onError(new Error('Connection timed out'))
+    req.abort()
+  })
+}
+
+const isRetryable = status => {
+  return (
+    status < 400 ||
+    status > 499 ||
+    [
+      408, // timeout
+      429 // too many requests
+    ].indexOf(status) !== -1)
 }