Internal error registry per package

changelog/@unreleased/pr-636.v2.yml

@@ -0,0 +1,12 @@
+type: feature
+feature:
+  description: |-
+    Replace global conjure-type error registration with a generated registry per conjure definition.
+
+    Creates a new `internal/conjureerrors` package in each output directory for conjure definitions that include errors. Generated client implementations provide the definition-specific registry as a ConjureErrorDecoder to be used when deserializing a non-2xx JSON response.
+
+    Clients will no longer be able to deserialize errors not defined in their own conjure definition.
+
+    Generating multiple IRs into the same output directory will result in all definitions sharing the same error registry.
+  links:
+  - https://github.com/palantir/conjure-go/pull/636

conjure/conjure.go

@@ -15,6 +15,7 @@
 package conjure
 
 import (
+	"path"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -46,31 +47,48 @@ func GenerateOutputFiles(conjureDefinition spec.ConjureDefinition, cfg OutputCon
 	}
 
 	var files []*OutputFile
+
+	var errorRegistryImportPath string
+	if len(conjureDefinition.Errors) > 0 {
+		errorRegistryImportPath, err = types.GetGoPackageForInternalErrors(cfg.OutputDir)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to determine import path for error registry package")
+		}
+		errorRegistryJenFile := jen.NewFilePathName(errorRegistryImportPath, path.Base(errorRegistryImportPath))
+		errorRegistryJenFile.ImportNames(snip.DefaultImportsToPackageNames)
+		writeErrorRegistryFile(errorRegistryJenFile.Group)
+		errorRegistryOutputDir, err := types.GetOutputDirectoryForGoPackage(cfg.OutputDir, errorRegistryImportPath)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to determine output directory for error registry package")
+		}
+		files = append(files, newGoFile(filepath.Join(errorRegistryOutputDir, "error_registry.conjure.go"), errorRegistryJenFile))
+	}
+
 	for _, pkg := range def.Packages {
 		if len(pkg.Aliases) > 0 {
-			aliasFile := newJenFile(pkg, def)
+			aliasFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, alias := range pkg.Aliases {
 				writeAliasType(aliasFile.Group, alias)
 			}
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "aliases.conjure.go"), aliasFile))
 		}
 		if len(pkg.Enums) > 0 {
-			enumFile := newJenFile(pkg, def)
+			enumFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, enum := range pkg.Enums {
 				writeEnumType(enumFile.Group, enum)
 			}
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "enums.conjure.go"), enumFile))
 		}
 		if len(pkg.Objects) > 0 {
-			objectFile := newJenFile(pkg, def)
+			objectFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, object := range pkg.Objects {
 				writeObjectType(objectFile.Group, object)
 			}
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "structs.conjure.go"), objectFile))
 		}
 		if len(pkg.Unions) > 0 {
-			unionFile := newJenFile(pkg, def)
-			goUnionGenericsFile := newJenFile(pkg, def)
+			unionFile := newJenFile(pkg, def, errorRegistryImportPath)
+			goUnionGenericsFile := newJenFile(pkg, def, errorRegistryImportPath)
 			goUnionGenericsFile.Comment("//go:build go1.18")
 			for _, union := range pkg.Unions {
 				writeUnionType(unionFile.Group, union, cfg.GenerateFuncsVisitor)
@@ -80,27 +98,27 @@ func GenerateOutputFiles(conjureDefinition spec.ConjureDefinition, cfg OutputCon
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "unions_generics.conjure.go"), goUnionGenericsFile))
 		}
 		if len(pkg.Errors) > 0 {
-			errorFile := newJenFile(pkg, def)
+			errorFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, errorDef := range pkg.Errors {
 				writeErrorType(errorFile.Group, errorDef)
 			}
-			astErrorInitFunc(errorFile.Group, pkg.Errors)
+			astErrorInitFunc(errorFile.Group, pkg.Errors, errorRegistryImportPath)
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "errors.conjure.go"), errorFile))
 		}
 		if len(pkg.Services) > 0 {
-			serviceFile := newJenFile(pkg, def)
+			serviceFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, service := range pkg.Services {
-				writeServiceType(serviceFile.Group, service)
+				writeServiceType(serviceFile.Group, service, errorRegistryImportPath)
 			}
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "services.conjure.go"), serviceFile))
 		}
 		if len(pkg.Services) > 0 && cfg.GenerateCLI {
-			cliFile := newJenFile(pkg, def)
+			cliFile := newJenFile(pkg, def, errorRegistryImportPath)
 			writeCLIType(cliFile.Group, pkg.Services)
 			files = append(files, newGoFile(filepath.Join(pkg.OutputDir, "cli.conjure.go"), cliFile))
 		}
 		if len(pkg.Services) > 0 && cfg.GenerateServer {
-			serverFile := newJenFile(pkg, def)
+			serverFile := newJenFile(pkg, def, errorRegistryImportPath)
 			for _, server := range pkg.Services {
 				writeServerType(serverFile.Group, server)
 			}
@@ -115,7 +133,7 @@ func GenerateOutputFiles(conjureDefinition spec.ConjureDefinition, cfg OutputCon
 	return files, nil
 }
 
-func newJenFile(pkg types.ConjurePackage, def *types.ConjureDefinition) *jen.File {
+func newJenFile(pkg types.ConjurePackage, def *types.ConjureDefinition, errorRegistryImportPath string) *jen.File {
 	f := jen.NewFilePathName(pkg.ImportPath, pkg.PackageName)
 	f.ImportNames(snip.DefaultImportsToPackageNames)
 	for _, conjurePackage := range def.Packages {
@@ -125,6 +143,9 @@ func newJenFile(pkg types.ConjurePackage, def *types.ConjureDefinition) *jen.Fil
 			f.ImportName(conjurePackage.ImportPath, conjurePackage.PackageName)
 		}
 	}
+	if errorRegistryImportPath != "" {
+		f.ImportName(errorRegistryImportPath, path.Base(errorRegistryImportPath))
+	}
 	return f
 }
 

conjure/errorwriter.go

@@ -486,10 +486,10 @@ func astErrorUnmarshalJSON(file *jen.Group, def *types.ErrorDefinition) {
 //	    errors.RegisterErrorType("MyNamespace:MyInternal", reflect.TypeOf(MyInternal{}))
 //	    errors.RegisterErrorType("MyNamespace:MyNotFound", reflect.TypeOf(MyNotFound{}))
 //	}
-func astErrorInitFunc(file *jen.Group, defs []*types.ErrorDefinition) {
+func astErrorInitFunc(file *jen.Group, defs []*types.ErrorDefinition, errorRegistryImportPath string) {
 	file.Func().Id("init").Params().BlockFunc(func(funcBody *jen.Group) {
 		for _, def := range defs {
-			funcBody.Add(snip.CGRErrorsRegisterErrorType()).Call(
+			funcBody.Qual(errorRegistryImportPath, "RegisterErrorType").Call(
 				jen.Lit(fmt.Sprintf("%s:%s", def.ErrorNamespace, def.Name)),
 				snip.ReflectTypeOf().Call(jen.Id(def.Name).Values()),
 			)
@@ -542,3 +542,28 @@ func selectorForErrorCode(errorCode spec.ErrorCode) *jen.Statement {
 		panic(fmt.Sprintf(`unknown error code string %q`, errorCode))
 	}
 }
+
+// writeErrorRegistryFile generates the error_registry.conjure.go file called
+// to register error types to a mapping that can be used by client methods.
+func writeErrorRegistryFile(file *jen.Group) {
+	file.Comment("Decoder returns the error type registry used by the conjure-generated")
+	file.Comment("clients in this package to convert JSON errors to their go types.")
+	file.Comment("Errors are registered by their error name. Only one type can be")
+	file.Comment("registered per name. If an error name is not recognized, a genericError")
+	file.Comment("is returned with all params marked unsafe.")
+	file.Func().Id("Decoder").Params().Params(snip.CGRErrorsConjureErrorDecoder()).Block(jen.Return(jen.Id("globalRegistry")))
+
+	file.Var().Id("globalRegistry").Op("=").Add(snip.CGRErrorsNewReflectTypeConjureErrorDecoder()).Call()
+
+	file.Comment("RegisterErrorType registers an error name and its go type in a global registry.")
+	file.Comment("The type should be a struct type whose pointer implements Error.")
+	file.Comment("Panics if name is already registered or *type does not implement Error.")
+	file.Func().Id("RegisterErrorType").Params(jen.Id("name").String(), jen.Id("typ").Qual("reflect", "Type")).Block(
+		jen.If(
+			jen.Err().Op(":=").Id("globalRegistry").Dot("RegisterErrorType").Call(jen.Id("name"), jen.Id("typ")),
+			jen.Err().Op("!=").Nil(),
+		).Block(
+			jen.Panic(jen.Err().Dot("Error").Call()),
+		),
+	)
+}

conjure/servicewriter.go

@@ -46,20 +46,20 @@ var (
 	pathParamRegexp = regexp.MustCompile(regexp.QuoteMeta("{") + "[^}]+" + regexp.QuoteMeta("}"))
 )
 
-func writeServiceType(file *jen.Group, serviceDef *types.ServiceDefinition) {
+func writeServiceType(file *jen.Group, serviceDef *types.ServiceDefinition, errorRegistryImportPath string) {
 	file.Add(astForServiceInterface(serviceDef, false, false))
 	file.Add(astForClientStructDecl(serviceDef.Name))
 	file.Add(astForNewClientFunc(serviceDef.Name))
 	for _, endpointDef := range serviceDef.Endpoints {
-		file.Add(astForEndpointMethod(serviceDef.Name, endpointDef, false))
+		file.Add(astForEndpointMethod(serviceDef.Name, endpointDef, errorRegistryImportPath, false))
 	}
 	if serviceDef.HasHeaderAuth() || serviceDef.HasCookieAuth() {
 		// at least one endpoint uses authentication: define decorator structures
 		file.Add(astForServiceInterface(serviceDef, true, false))
 		file.Add(astForNewServiceFuncWithAuth(serviceDef))
 		file.Add(astForClientStructDeclWithAuth(serviceDef))
 		for _, endpointDef := range serviceDef.Endpoints {
-			file.Add(astForEndpointMethod(serviceDef.Name, endpointDef, true))
+			file.Add(astForEndpointMethod(serviceDef.Name, endpointDef, errorRegistryImportPath, true))
 		}
 
 		// Return true if all endpoints that require authentication are of the same auth type (header or cookie) and at least
@@ -207,7 +207,7 @@ func astForNewServiceFuncWithAuth(serviceDef *types.ServiceDefinition) *jen.Stat
 		))
 }
 
-func astForEndpointMethod(serviceName string, endpointDef *types.EndpointDefinition, withAuth bool) *jen.Statement {
+func astForEndpointMethod(serviceName string, endpointDef *types.EndpointDefinition, errorRegistryImportPath string, withAuth bool) *jen.Statement {
 	return jen.Func().
 		ParamsFunc(func(receiver *jen.Group) {
 			if withAuth {
@@ -227,12 +227,12 @@ func astForEndpointMethod(serviceName string, endpointDef *types.EndpointDefinit
 			if withAuth {
 				astForEndpointAuthMethodBodyFunc(methodBody, endpointDef)
 			} else {
-				astForEndpointMethodBodyFunc(methodBody, endpointDef)
+				astForEndpointMethodBodyFunc(methodBody, endpointDef, errorRegistryImportPath)
 			}
 		})
 }
 
-func astForEndpointMethodBodyFunc(methodBody *jen.Group, endpointDef *types.EndpointDefinition) {
+func astForEndpointMethodBodyFunc(methodBody *jen.Group, endpointDef *types.EndpointDefinition, errorRegistryImportPath string) {
 	var (
 		hasReturnVal         = endpointDef.Returns != nil
 		returnsBinary        = hasReturnVal && (*endpointDef.Returns).IsBinary()
@@ -266,7 +266,7 @@ func astForEndpointMethodBodyFunc(methodBody *jen.Group, endpointDef *types.Endp
 	}
 
 	// build requestParams
-	astForEndpointMethodBodyRequestParams(methodBody, endpointDef)
+	astForEndpointMethodBodyRequestParams(methodBody, endpointDef, errorRegistryImportPath)
 
 	// execute request
 	callStmt := jen.Id(clientReceiverName).Dot(clientStructFieldName).Dot("Do").Call(
@@ -323,7 +323,7 @@ func astForEndpointMethodBodyFunc(methodBody *jen.Group, endpointDef *types.Endp
 	}
 }
 
-func astForEndpointMethodBodyRequestParams(methodBody *jen.Group, endpointDef *types.EndpointDefinition) {
+func astForEndpointMethodBodyRequestParams(methodBody *jen.Group, endpointDef *types.EndpointDefinition, errorRegistryImportPath string) {
 	methodBody.Var().Id(requestParamsVar).Op("[]").Add(snip.CGRClientRequestParam())
 
 	// helper for the statement "requestParams = append(requestParams, {code})"
@@ -427,6 +427,10 @@ func astForEndpointMethodBodyRequestParams(methodBody *jen.Group, endpointDef *t
 			appendRequestParams(methodBody, snip.CGRClientWithJSONResponse().Call(jen.Op("&").Id(returnValVar)))
 		}
 	}
+	// errors
+	if errorRegistryImportPath != "" {
+		appendRequestParams(methodBody, snip.CGRClientWithRequestConjureErrorDecoder().Call(jen.Qual(errorRegistryImportPath, "Decoder").Call()))
+	}
 }
 
 func astForEndpointAuthMethodBodyFunc(methodBody *jen.Group, endpointDef *types.EndpointDefinition) {