Merge pull request #2307 from overleaf/spd-project-page-without-v1

Add additional error handling to enable /projects to load without V1

GitOrigin-RevId: 710ab2f07f191aa60ffdd71e2f54bc7c5db0c430

services/web/app/src/Features/Institutions/InstitutionsAPI.js

@@ -17,6 +17,7 @@ const settings = require('settings-sharelatex')
 const request = require('request')
 const { promisifyAll } = require('../../util/promises')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
+const { V1ConnectionError } = require('../Errors/Errors')
 
 const InstitutionsAPI = {
   getInstitutionAffiliations(institutionId, callback) {
@@ -205,7 +206,22 @@ var makeAffiliationRequest = function(requestOptions, callback) {
     },
     function(error, response, body) {
       if (error != null) {
-        return callback(error)
+        return callback(
+          new V1ConnectionError('error getting affiliations from v1').withCause(
+            error
+          )
+        )
+      }
+      if (response && response.statusCode >= 500) {
+        return callback(
+          new V1ConnectionError({
+            message: 'error getting affiliations from v1',
+            info: {
+              status: response.statusCode,
+              body: body
+            }
+          })
+        )
       }
       let isSuccess = response.statusCode >= 200 && response.statusCode < 300
       if (!isSuccess) {

services/web/app/src/Features/Project/ProjectController.js

@@ -364,6 +364,7 @@ const ProjectController = {
     const timer = new metrics.Timer('project-list')
     const userId = AuthenticationController.getLoggedInUserId(req)
     const currentUser = AuthenticationController.getSessionUser(req)
+    let noV1Connection = false
     async.parallel(
       {
         tags(cb) {
@@ -389,7 +390,8 @@ const ProjectController = {
               projects = []
             }
             if (error != null && error instanceof V1ConnectionError) {
-              return cb(null, { projects: [], tags: [], noConnection: true })
+              noV1Connection = true
+              return cb(null, null)
             }
             cb(error, projects[0])
           })
@@ -399,6 +401,7 @@ const ProjectController = {
             currentUser,
             (error, hasPaidSubscription) => {
               if (error != null && error instanceof V1ConnectionError) {
+                noV1Connection = true
                 return cb(null, true)
               }
               cb(error, hasPaidSubscription)
@@ -416,7 +419,13 @@ const ProjectController = {
           if (!Features.hasFeature('affiliations')) {
             return cb(null, null)
           }
-          getUserAffiliations(userId, cb)
+          getUserAffiliations(userId, (error, affiliations) => {
+            if (error && error instanceof V1ConnectionError) {
+              noV1Connection = true
+              return cb(null, [])
+            }
+            cb(error, affiliations)
+          })
         }
       },
       (err, results) => {
@@ -424,6 +433,10 @@ const ProjectController = {
           logger.warn({ err }, 'error getting data for project list page')
           return next(err)
         }
+        if (noV1Connection) {
+          results.v1Projects = results.v1Projects || { projects: [], tags: [] }
+          results.v1Projects.noConnection = true
+        }
         const { notifications, user, userAffiliations } = results
         const v1Tags =
           (results.v1Projects != null ? results.v1Projects.tags : undefined) ||
@@ -995,8 +1008,10 @@ const ProjectController = {
       v1ProjectData = {}
     }
     const warnings = []
-    if (v1ProjectData.noConnection && Settings.overleaf) {
-      warnings.push('No V1 Connection')
+    if (v1ProjectData.noConnection) {
+      warnings.push(
+        'Error accessing Overleaf V1. Some of your projects or features may be missing.'
+      )
     }
     if (v1ProjectData.hasHiddenV1Projects) {
       warnings.push(

services/web/app/src/Features/Subscription/LimitationsManager.js

@@ -21,6 +21,7 @@ const Settings = require('settings-sharelatex')
 const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const CollaboratorsInvitesHandler = require('../Collaborators/CollaboratorsInviteHandler')
 const V1SubscriptionManager = require('./V1SubscriptionManager')
+const { V1ConnectionError } = require('../Errors/Errors')
 
 module.exports = LimitationsManager = {
   allowedNumberOfCollaboratorsInProject(project_id, callback) {
@@ -107,7 +108,11 @@ module.exports = LimitationsManager = {
           }
           return this.userHasV1Subscription(user, (err, hasV1Subscription) => {
             if (err != null) {
-              return callback(err)
+              return callback(
+                new V1ConnectionError(
+                  'error getting subscription from v1'
+                ).withCause(err)
+              )
             }
             logger.log(
               {

services/web/app/src/Features/Subscription/V1SubscriptionManager.js

@@ -180,11 +180,20 @@ module.exports = V1SubscriptionManager = {
         },
         function(error, response, body) {
           if (error != null) {
-            // Specially handle no connection err, so warning can be shown
-            if (error.code === 'ECONNREFUSED') {
-              error = new V1ConnectionError('No V1 connection')
-            }
-            return callback(error)
+            return callback(
+              new V1ConnectionError('no v1 connection').withCause(error)
+            )
+          }
+          if (response && response.statusCode >= 500) {
+            return callback(
+              new V1ConnectionError({
+                message: 'error from v1',
+                info: {
+                  status: response.statusCode,
+                  body: body
+                }
+              })
+            )
           }
           if (response.statusCode >= 200 && response.statusCode < 300) {
             return callback(null, body, v1Id)

services/web/app/src/Features/V1/V1Api.js

@@ -61,7 +61,17 @@ module.exports = V1Api = {
 
   _responseHandler(options, error, response, body, callback) {
     if (error != null) {
-      return callback(error, response, body)
+      return callback(
+        new Errors.V1ConnectionError('error from V1 API').withCause(error)
+      )
+    }
+    if (response && response.statusCode >= 500) {
+      return callback(
+        new Errors.V1ConnectionError({
+          message: 'error from V1 API',
+          info: { status: response.statusCode, body: body }
+        })
+      )
     }
     if (
       (response.statusCode >= 200 && response.statusCode < 300) ||

services/web/test/unit/src/Institutions/InstitutionsAPITests.js

@@ -21,6 +21,7 @@ const modulePath = path.join(
   '../../../../app/src/Features/Institutions/InstitutionsAPI'
 )
 ;({ expect } = require('chai'))
+const Errors = require('../../../../app/src/Features/Errors/Errors')
 
 describe('InstitutionsAPI', function() {
   beforeEach(function() {
@@ -46,7 +47,8 @@ describe('InstitutionsAPI', function() {
         },
         '../../../../../app/src/Features/V1/V1Api': {
           request: sinon.stub()
-        }
+        },
+        '../Errors/Errors': Errors
       }
     })
 
@@ -158,9 +160,7 @@ describe('InstitutionsAPI', function() {
       return this.InstitutionsAPI.getUserAffiliations(
         this.stubbedUser._id,
         err => {
-          should.exist(err)
-          err.message.should.have.string(503)
-          err.message.should.have.string(body.errors)
+          expect(err).to.be.instanceof(Errors.V1ConnectionError)
           return done()
         }
       )
@@ -299,9 +299,7 @@ describe('InstitutionsAPI', function() {
       return this.InstitutionsAPI.deleteAffiliations(
         this.stubbedUser._id,
         err => {
-          should.exist(err)
-          err.message.should.have.string(518)
-          err.message.should.have.string(body.errors)
+          expect(err).to.be.instanceof(Errors.V1ConnectionError)
           return done()
         }
       )

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -520,6 +520,48 @@ describe('ProjectController', function() {
       this.ProjectController.projectListPage(this.req, this.res)
     })
 
+    describe('when there is a v1 connection error', function() {
+      beforeEach(function() {
+        this.Features.hasFeature = sinon
+          .stub()
+          .withArgs('overleaf-integration')
+          .returns(true)
+        this.connectionWarning =
+          'Error accessing Overleaf V1. Some of your projects or features may be missing.'
+      })
+
+      it('should show a warning when there is an error getting v1 projects', function(done) {
+        this.Modules.hooks.fire
+          .withArgs('findAllV1Projects', this.user._id)
+          .yields(new Errors.V1ConnectionError('error'))
+        this.res.render = (pageName, opts) => {
+          expect(opts.warnings).to.contain(this.connectionWarning)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
+      })
+
+      it('should show a warning when there is an error getting subscriptions from v1', function(done) {
+        this.LimitationsManager.hasPaidSubscription.yields(
+          new Errors.V1ConnectionError('error')
+        )
+        this.res.render = (pageName, opts) => {
+          expect(opts.warnings).to.contain(this.connectionWarning)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
+      })
+
+      it('should show a warning when there is an error getting affiliations from v1', function(done) {
+        this.getUserAffiliations.yields(new Errors.V1ConnectionError('error'))
+        this.res.render = (pageName, opts) => {
+          expect(opts.warnings).to.contain(this.connectionWarning)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
+      })
+    })
+
     describe('front widget', function(done) {
       beforeEach(function() {
         this.settings.overleaf = {