Comments (3)
The issue is that the generated errors.conjure.go
file generates the following code:
func init() {
errors.RegisterErrorType("V6:EntityNotFound", reflect.TypeOf(EntityNotFound{}))
}
The errors.RegisterErrorType
function uses shared storage and panics if registration is done with the same key.
In the steps above, the repository ends with with 2 copies of the same errors.conjure.go
(one in the consumed/vendored module and one in the local module), so the init
function is called twice, which causes the panic at runtime.
It is an intentional design decision to require that error types be globally unique, but this poses a problem in the above scenario. In such a scenario, currently the only workaround is to modify the generated code (basically, to remove the error registration). Although this works, it involves manual edits and also makes it such that typed errors aren't returned for the locally generated code.
Possible Solutions
Associate error registration with particular client
The purpose of registering the errors is to allow conjure-go-runtime
to provide a strongly typed error -- it maps the unmarshal of the error to the type provided.
The problem in this circumstance is that there are now 2 separate Go types that both map to the same error. However, in most cases the "scope" should be fairly well-defined -- any calls that use the vendored version of the type (declare the vendored error type as a return error type or operate on it) should use that type, while "local" code should use the local type.
Right now conjure-go-runtime
is designed to have a single global lookup table for errors, but theoretically we could namespace this. For example, we could modify the error registration function so that the defining module is provided when registering errors, and could stipulate that typed errors would only be supported if a client is instantiated with a module.
As an example of this proposal, the current function signature:
func RegisterErrorType(name string, typ reflect.Type)
Could be changed to:
func RegisterErrorType(registeringModule, name string, typ reflect.Type)
Then the generated init
code for the 2 modules in the example above would be:
func init() {
errors.RegisterErrorType("github.com/palantir/test-library-module", "V6:EntityNotFound", reflect.TypeOf(EntityNotFound{}))
}
func init() {
errors.RegisterErrorType("github.com/palantir/test-service-module", "V6:EntityNotFound", reflect.TypeOf(EntityNotFound{}))
}
We could then change the conjure-go-runtime
client initialization to either require specifying a module or attempt to infer the module (although not sure how practical the latter would be).
It's not totally clear that this would be the best approach, but it's an illustration of a possible way to solve this general issue.
from conjure-go.
Updates
I looked into Nick's suggested possible solution today and drafted the beginnings of something I think will work:
conjure-go changes
I opened this PR to add "conjuremoduleregistrar" package generation. This adds one package per conjure project, where the contents of the package record the package's path:
package conjuremoduleregistrar
import (
"runtime"
)
var ConjureModuleIdentifier string
func init() {
_, ConjureModuleIdentifier, _, _ = runtime.Caller(0)
}
The value of ConjureModuleIdentifier will depend on the root path of the conjure package, letting us discern between vendored and locally generated code.
conjure-go-runtime changes
I opened this PR adding support for per-module error registries, as well as method for storing retrieving module names from contexts. A later conjure-go PR can use this new method in the following way:
func init() {
errors.RegisterErrorTypeForModule(conjuremoduleregistrar.ConjureModuleIdentifier, errName, errType)
}
If vendored conjure and locally generated conjure update to use this instead of the existing RegisterErrorType method, they can deconflict their error registration.
still to do
The following is the remaining work I'm aware of in terms of getting this wired up fully:
- update conjure-go's client generation such that generated clients add the conjure module as a context parameter:
request = request.WithContext(errors.WithContextErrorRegistryModuleName(conjuremoduleregistrar.ConjureModuleIdentifier)
- update CGR's response error decoder middleware to pull this module name off of request contexts so that it can use the appropriate error registry:
moduleName := errors.ErrorRegistryFromContext(resp.Request.Context())
conjureErr, err := errors.UnmarshalErrorForModule(moduleName, respBodyBytes)
I don't love using request contexts for wiring here so I'm open to any alternatives people think of.
from conjure-go.
Given the global error registry is strictly used for CGR clients to know how to unmarshal a given error type, what if we instead defined a new interface for a conjure error decoder that knows how to unmarshal a specific/strongly-typed error and return a conjure error. Something like this that takes in the name of the error and the body and returns a conjure error. The name is already parsed out in the errors.UnmarshalError()
func (code) which is where we could call the conjure error decoder. Alternatively, we could move the name
unmarshaling into the generated conjure error decoder and change the signature slightly.
type ConjureErrorDecoder interface {
DecodeConjureError(name string, body []byte) (errors.Error, error)
}
Then we can generate an implementation of this ConjureErrorDecoder
alongside the error definitions which can be wired into the generated service clients.
func (e errorDecoder) DecodeConjureError(name string, body []byte) (errors.Error, error) {
switch name {
case "V6:EntityNotFound":
var entityNotFoundErr EntityNotFound
if err := codecs.JSON.Unmarshal(body, &entityNotFoundErr); err != nil {
return nil, err
}
return &entityNotFoundErr, nil
return nil, errors.New("Unsupported type")
}
Example service client that wires in the ConjureErrorDecoder above.
func (c *v6Client) GetEntity(ctx context.Context, authHeader bearertoken.Token, requestArg GetEntityRequest) (EntityResp, error) {
var defaultReturnVal EntityResp
var returnVal *EntityResp
var requestParams []httpclient.RequestParam
requestParams = append(requestParams, httpclient.WithRPCMethodName("GetEntity"))
requestParams = append(requestParams, httpclient.WithRequestMethod("GET"))
requestParams = append(requestParams, httpclient.WithPathf("/entity"))
requestParams = append(requestParams, httpclient.WithJSONRequest(requestArg))
requestParams = append(requestParams, httpclient.WithJSONResponse(&returnVal))
// This is where we wire in the error decoder above.
requestParams = append(requestParams, httpclient.WithRequestConjureErrorDecoder(errorDecoder{}))
if _, err := c.client.Do(ctx, requestParams...); err != nil {
return defaultReturnVal, werror.WrapWithContextParams(ctx, err, "getEntity failed")
}
if returnVal == nil {
return defaultReturnVal, werror.ErrorWithContextParams(ctx, "getEntity response cannot be nil")
}
return *returnVal, nil
}
where the WithRequestConjureErrorDecoder
wraps the restErrorDecoder
to pass in the new conjure error decoder.
func WithRequestConjureErrorDecoder(ced ConjureErrorDecoder) RequestParam {
return requestParamFunc(func(b *requestBuilder) error {
b.errorDecoderMiddleware = errorDecoderMiddleware{
errorDecoder: restErrorDecoder{
conjureErrorDecoder: ced,
},
}
return nil
})
}
This allows us to remove the global registry altogether and more tightly couples the errors defined to the endpoints that use them. One edge case with this approach is if users define the set of errors and use them in a service of differing pkgs given the conjure error decoder is pkg private, which might be a non-starter for this approach.
Longer term I would love to be able to define what errors are returned from any defined conjure endpoint, similar to how OpenAPI works, but this is not yet supported in conjure.
from conjure-go.
Related Issues (20)
- Integer should be generated as an int32 HOT 3
- Allow for marking endpoint arguments Safe
- client: Verify path parameters are non-empty before sending request HOT 4
- verification failure: Clients should error if response is missing required fields HOT 15
- Bad request response for lists and sets as query parameters HOT 1
- Conjure errors should have comparison helper function HOT 5
- Server implementations do not compile when using type-hinted java external imports
- Generated code does not pass linting HOT 2
- Improved handling for deprecated endpoints and objects
- Safe and Unsafe arg names can collide with method names on an error HOT 2
- empty optional<binary> should return as a 204 status code
- Iterable Binary return type inconsistent with other binary types HOT 2
- Break in code-gen for external types used in path/query params HOT 1
- Alias of an external type with an any fallback produces non-compiling code
- Managed Go version is not used in CircleCI
- CircleCI config includes failing publish step to Bintray
- Use generics to improve Ergonomics of generated union types HOT 2
- Generated code does not compile if Conjure definition references type in a package where last element is of the form "v[0-9]+" HOT 3
- Generated union code panics when marshalling union type with `nil` member HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from conjure-go.