Uploaded image for project: 'Couchbase Gateway'
  1. Couchbase Gateway
  2. CBG-306

Avoid panics as an error handling mechanism

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • Major
    • 2.6.0
    • None
    • SyncGateway
    • Security Level: Public
    • None
    • CBG Sprint 20, CBG Sprint 21
    • 5

    Description

      There are a number of places where Sync Gateway might panic:

      $ rg 'panic\(' -g '!*test.go' -g '!*testing.go' -c
      auth/user.go:3
      auth/role.go:1
      db/revision_cache.go:2
      db/revision.go:1
      db/document.go:1
      db/revtree.go:3
      channels/set.go:2
      base/bucket.go:2
      channels/change_log.go:1
      base/logging.go:1
      base/logger_console.go:1
      base/util.go:1
      rest/handler.go:1
      base/sequence_clock.go:1
      rest/bulk_api.go:1
      rest/server_context.go:2
      service/sg-windows/sg-accel-service/sg-accel-service.go:2
      service/sg-windows/sg-service/sg-service.go:2
      

      Whilst some of these might be legitimate, others should be substituted for error handling and propagation, so that we can isolate a particular failure to the operation/connection/replication that it affects and not bring the whole process down.

      A prime example would be in auth.(*userImpl).GetRoles:

      func (user *userImpl) GetRoles() []Role {
      	if user.roles == nil {
      		roles := make([]Role, 0, len(user.RolesSince_))
      		for name := range user.RolesSince_ {
      			role, err := user.auth.GetRole(name)
      			//base.Infof(base.KeyAccess, "User %s role %q = %v", base.UD(user.Name_), base.UD(name), base.UD(role))
      			if err != nil {
      				panic(fmt.Sprintf("Error getting user role %q: %v", name, err))
      			} else if role != nil {
      				roles = append(roles, role)
      			}
      		}
      		user.roles = roles
      	}
      	return user.roles
      }
      

      Raising this CBG as a high level ticket to potentially run through each of the panics listed above and comb out those that should be changed.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            jacques.rascagneres Jacques Rascagneres
            James Flather James Flather (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty