Handle sync network failure errors

app/extensions.js

@@ -225,15 +225,18 @@ let generateSyncManifest = () => {
     'default-src': '\'self\'',
     'form-action': '\'none\''
   }
-  cspDirectives['connect-src'] = ['\'self\'',
-    appConfig.sync.serverUrl,
-    appConfig.sync.s3Url].join(' ')
+  const connectSources = ['\'self\'', appConfig.sync.serverUrl, appConfig.sync.s3Url]
+  if (process.env.NODE_ENV === 'development') {
+    connectSources.push(appConfig.sync.testS3Url)
+  }
+
+  cspDirectives['connect-src'] = connectSources.join(' ')
 
   if (process.env.NODE_ENV === 'development') {
     // allow access to webpack dev server resources
     let devServer = 'localhost:' + process.env.npm_package_config_port
     cspDirectives['default-src'] += ' http://' + devServer
-    cspDirectives['connect-src'] += ' http://' + devServer + ' ws://' + devServer + ' ' + appConfig.sync.testS3Url
+    cspDirectives['connect-src'] += ' http://' + devServer + ' ws://' + devServer
   }
 
   return {

app/extensions/brave/locales/en-US/app.properties

@@ -257,3 +257,5 @@ rememberThisDecision=Remember this decision for {{origin}}
 copyToClipboard.title=Copy to clipboard
 preventMoreAlerts=Prevent this page from creating additional dialogs
 copied=Copied!
+connectionError=Server connection failed. Please make sure you are connected to the Internet.
+unknownError=Oops, something went wrong.

app/extensions/brave/locales/en-US/preferences.properties

@@ -40,6 +40,7 @@ syncNewDevice=Sync a new device…
 syncClearProfile=Sync a new device…
 syncClearData=Clear Data
 syncResetButton=Reset Sync…
+syncRetryButton=Try again
 syncResetDataDisabled=This feature is only available when Sync is enabled.
 syncReset=Reset Sync
 syncResetMessageWhat=Resetting Sync clears data stored on the Sync server and resets this device's Sync settings.

app/locale.js

@@ -239,7 +239,9 @@ var rendererIdentifiers = function () {
     'importSuccess',
     'licenseTextOk',
     'closeFirefoxWarningOk',
-    'importSuccessOk'
+    'importSuccessOk',
+    'connectionError',
+    'unknownError'
   ]
 }
 

app/renderer/components/preferences/syncTab.js

@@ -27,14 +27,25 @@ class SyncTab extends ImmutableComponent {
     this.enableRestore = this.enableRestore.bind(this)
   }
 
+  get setupError () {
+    return this.props.syncData.get('setupError')
+  }
+
   get isSetup () {
-    return this.props.syncData.get('seed') instanceof Immutable.List && this.props.syncData.get('seed').size === 32
+    return !this.setupError && this.props.syncData.get('seed') instanceof Immutable.List && this.props.syncData.get('seed').size === 32
   }
 
   get enabled () {
     return getSetting(settings.SYNC_ENABLED, this.props.settings)
   }
 
+  get errorContent () {
+    return <div className='errorContainer'>
+      <div className='setupError'>{this.setupError}</div>
+      <Button l10nId='syncRetryButton' className='primaryButton' onClick={this.retry.bind(this)} />
+    </div>
+  }
+
   get clearDataContent () {
     return <div className='syncClearData'>
       <div className='sectionTitle' data-l10n-id='syncClearData' />
@@ -50,6 +61,9 @@ class SyncTab extends ImmutableComponent {
   }
 
   get setupContent () {
+    if (this.setupError) {
+      return null
+    }
     // displayed before a sync userId has been created
     return <div className='setupContent'>
       <Button l10nId='syncStart' className='primaryButton' onClick={this.props.showOverlay.bind(this, 'syncStart')} />
@@ -210,6 +224,11 @@ class SyncTab extends ImmutableComponent {
     }
   }
 
+  retry () {
+    aboutActions.reloadSyncExtension()
+    window.location.reload()
+  }
+
   setupSyncProfile (isRestoring) {
     this.props.onChangeSetting(settings.SYNC_DEVICE_NAME,
       this.deviceNameInput.value || this.defaultDeviceName)
@@ -272,7 +291,9 @@ class SyncTab extends ImmutableComponent {
           <span className='fa fa-question-circle fundsFAQ' />
         </a>
         {
-          this.isSetup
+          this.setupError
+          ? this.errorContent
+          : this.isSetup
             ? this.postSetupContent
             : this.setupContent
         }

app/sync.js

@@ -8,6 +8,7 @@ const Immutable = require('immutable')
 const electron = require('electron')
 const qr = require('qr-image')
 const ipcMain = electron.ipcMain
+const locale = require('./locale')
 const messages = require('../js/constants/sync/messages')
 const categories = require('../js/constants/sync/proto').categories
 const writeActions = require('../js/constants/sync/proto').actions
@@ -160,6 +161,7 @@ const doAction = (sender, action) => {
  * @param {Event} e
  */
 module.exports.onSyncReady = (isFirstRun, e) => {
+  appActions.setSyncSetupError(null)
   if (!syncEnabled()) {
     return
   }
@@ -285,6 +287,8 @@ module.exports.init = function (appState) {
   })
   // GET_INIT_DATA is the first message sent by the sync-client when it starts
   ipcMain.on(messages.GET_INIT_DATA, (e) => {
+    // Clear any old errors
+    appActions.setSyncSetupError(null)
     // Unregister the previous dispatcher cb
     if (dispatcherCallback) {
       appDispatcher.unregister(dispatcherCallback)
@@ -297,7 +301,14 @@ module.exports.init = function (appState) {
       const appState = AppStore.getState().get('sync')
       const seed = appState.get('seed') ? Array.from(appState.get('seed')) : null
       deviceId = appState.get('deviceId') ? Array.from(appState.get('deviceId')) : null
-      e.sender.send(messages.GOT_INIT_DATA, seed, deviceId, config)
+      const syncConfig = {
+        apiVersion: config.apiVersion,
+        debug: config.debug,
+        serverUrl: getSetting(settings.SYNC_NETWORK_DISABLED)
+          ? 'http://localhost' // set during tests to simulate network failure
+          : config.serverUrl
+      }
+      e.sender.send(messages.GOT_INIT_DATA, seed, deviceId, syncConfig)
     }
   })
   // SAVE_INIT_DATA is sent by about:preferences before sync is enabled
@@ -337,6 +348,13 @@ module.exports.init = function (appState) {
   ipcMain.on(messages.SYNC_DEBUG, (e, msg) => {
     log(msg)
   })
+  ipcMain.on(messages.SYNC_SETUP_ERROR, (e, error) => {
+    if (error === 'Failed to fetch') {
+      // This is probably the most common error, so give it a more useful message.
+      error = locale.translation('connectionError')
+    }
+    appActions.setSyncSetupError(error || locale.translation('unknownError'))
+  })
   ipcMain.on(messages.GET_EXISTING_OBJECTS, (event, categoryName, records) => {
     if (!syncEnabled()) {
       return
@@ -381,4 +399,5 @@ module.exports.stop = function () {
   if (pollIntervalId !== null) {
     clearInterval(pollIntervalId)
   }
+  appActions.setSyncSetupError(null)
 }

docs/appActions.md

@@ -722,6 +722,16 @@ Dispatches a message when sync init data needs to be saved
 
 
 
+### setSyncSetupError(error) 
+
+Sets the sync setup error, or null for no error.
+
+**Parameters**
+
+**error**: `string | null`, Sets the sync setup error, or null for no error.
+
+
+
 ### applySiteRecords(records) 
 
 Dispatches a message to apply a batch of site records from Brave Sync

docs/state.md

@@ -275,6 +275,7 @@ AppStore
     },
     seed: Array.<number>,
     seedQr: string, // data URL of QR code representing the seed
+    setupError: string? // indicates that an error occurred during sync setup
   },
   tabs: [{
     // persistent properties

js/actions/appActions.js

@@ -882,6 +882,17 @@ const appActions = {
     })
   },
 
+  /**
+   * Sets the sync setup error, or null for no error.
+   * @param {string|null} error
+   */
+  setSyncSetupError: function (error) {
+    AppDispatcher.dispatch({
+      actionType: appConstants.APP_SET_SYNC_SETUP_ERROR,
+      error
+    })
+  },
+
   /**
    * Dispatches a message to apply a batch of site records from Brave Sync
    * TODO: Refactor this to merge it into addSite/removeSite

js/constants/appConfig.js

@@ -168,6 +168,7 @@ module.exports = {
     // sync
     'sync.enabled': false,
     'sync.device-name': 'browser-laptop',
+    'sync.network.disabled': false,
     'sync.type.bookmark': true,
     'sync.type.history': false,
     'sync.type.siteSetting': true,

js/constants/appConstants.js

@@ -94,6 +94,7 @@ const appConstants = {
   APP_CREATE_SYNC_CACHE: _,
   APP_SAVE_SYNC_INIT_DATA: _,
   APP_RESET_SYNC_DATA: _,
+  APP_SET_SYNC_SETUP_ERROR: _,
   APP_ADD_NOSCRIPT_EXCEPTIONS: _,
   APP_TAB_MESSAGE_BOX_SHOWN: _,
   APP_TAB_MESSAGE_BOX_DISMISSED: _,

js/constants/settings.js

@@ -75,6 +75,7 @@ const settings = {
   SYNC_TYPE_BOOKMARK: 'sync.type.bookmark',
   SYNC_TYPE_HISTORY: 'sync.type.history',
   SYNC_TYPE_SITE_SETTING: 'sync.type.siteSetting',
+  SYNC_NETWORK_DISABLED: 'sync.network.disabled', // disable network connection to sync server. only used in testing.
   // DEPRECATED settings
   // ########################
   // these constants should not appear outside of this file, ../settings.js, and our tests

js/constants/sync/messages.js

@@ -16,6 +16,12 @@ const messages = {
    * easily accessible, such as in browser-laptop.
    */
   SYNC_DEBUG: _, /* @param {string} message */
+  /**
+   * webview -> browser
+   * indicates that a fatal error occurred during sync setup, meaning that sync
+   * is not running.
+   */
+  SYNC_SETUP_ERROR: _, /* @param {string} error */
   /**
    * webview -> browser
    * browser sends GOT_INIT_DATA with the saved values

js/stores/appStore.js

@@ -898,6 +898,9 @@ const handleAppAction = (action) => {
         appState = appState.setIn(['sync', 'seedQr'], action.seedQr)
       }
       break
+    case appConstants.APP_SET_SYNC_SETUP_ERROR:
+      appState = appState.setIn(['sync', 'setupError'], action.error)
+      break
     case appConstants.APP_CREATE_SYNC_CACHE:
       appState = syncUtil.createSiteCache(appState)
       break

less/about/preferences.less

@@ -632,9 +632,13 @@ table.sortableTable {
   .browserButton + .browserButton {
     margin-left: 0.75em;
   }
-  .setupContent {
+  .setupContent, .setupError {
     margin: 1em 0;
   }
+  .setupError {
+    color: red;
+    font-weight: bold;
+  }
   .deviceNameInput {
     width: 50%;
   }

test/misc-components/syncTest.js

@@ -193,6 +193,50 @@ describe('Sync Panel', function () {
     })
   })
 
+  describe('sync setup failure', function () {
+    Brave.beforeAll(this)
+    before(function * () {
+      yield setup(this.app.client)
+    })
+
+    it('shows error when sync fails', function * () {
+      yield this.app.client
+        .changeSetting(settings.SYNC_NETWORK_DISABLED, true)
+        .tabByIndex(0)
+        .loadUrl(prefsUrl)
+        .waitForVisible(syncTab)
+        .click(syncTab)
+        .waitForVisible(startButton)
+        .click(startButton)
+        .waitForVisible(createButton)
+        .click(createButton)
+        .waitUntil(function () {
+          return this.getText('.setupError').then((val) => {
+            return val.includes('connection failed')
+          })
+        })
+        .windowByUrl(Brave.browserWindowUrl)
+    })
+
+    it('can retry sync connection', function * () {
+      const retryButton = '[data-l10n-id="syncRetryButton"]'
+      yield this.app.client
+        .changeSetting(settings.SYNC_NETWORK_DISABLED, true)
+        .tabByIndex(0)
+        .loadUrl(prefsUrl)
+        .waitForVisible(syncTab)
+        .click(syncTab)
+        .waitForVisible(retryButton)
+        .click(retryButton)
+        .windowByUrl(Brave.browserWindowUrl)
+        .changeSetting(settings.SYNC_NETWORK_DISABLED, false)
+        .tabByIndex(0)
+        .waitForVisible(retryButton)
+        .click(retryButton)
+        .waitForVisible(syncSwitch)
+    })
+  })
+
   describe('sync post-setup', function () {
     Brave.beforeEach(this)
     beforeEach(function * () {