Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic vs Returning err #249

Closed
mgirouard opened this issue Sep 4, 2020 · 2 comments
Closed

Panic vs Returning err #249

mgirouard opened this issue Sep 4, 2020 · 2 comments

Comments

@mgirouard
Copy link
Contributor

mgirouard commented Sep 4, 2020

I'm exploring libraries for a JSON Hyperschema to OpenAPI transition we're doing. kin-openapi seems to meet a lot of our needs. When testing some request and response validation, I was surprised to get a panic from my vendor code.

go run main.go
panic: Encountered non-allowed external reference: 'zone/ssl/networks.json#/components/schemas/networks'

goroutine 1 [running]:
github.com/getkin/kin-openapi/openapi3filter.(*Router).WithSwaggerFromFile(...)
        /home/cf/pkg/mod/github.com/getkin/kin-openapi@v0.20.0/openapi3filter/router.go:74
main.main()
        /home/cf/src/json-schema-to-openapi-schema/go/main.go:15 +0x6a5
exit status 2

Looking more, it seems like panic is fairly heavily used and I was hoping to understand the reasoning behind panicing over simply returning an error. This makes it really difficult to use as a library.

$ grep -nir panic
github.com/getkin/kin-openapi/jsoninfo/marshal.go:61:           // Panic because this is a clear programming error
github.com/getkin/kin-openapi/jsoninfo/marshal.go:62:           panic(fmt.Errorf("Value %s is not a pointer", reflection.Type().String()))
github.com/getkin/kin-openapi/jsoninfo/marshal.go:65:           // Panic because this is a clear programming error
github.com/getkin/kin-openapi/jsoninfo/marshal.go:66:           panic(fmt.Errorf("Value %s is nil", reflection.Type().String()))
github.com/getkin/kin-openapi/jsoninfo/marshal.go:149:                  panic(fmt.Errorf("Field '%s' has unsupported type %s", field.JSONName, field.Type.String()))
github.com/getkin/kin-openapi/jsoninfo/unmarshal.go:44:         panic(fmt.Errorf("Value %T is not a pointer", value))
github.com/getkin/kin-openapi/jsoninfo/unmarshal.go:47:         panic(fmt.Errorf("Value %T is nil", value))
github.com/getkin/kin-openapi/jsoninfo/unmarshal.go:55:         panic(fmt.Errorf("Value %T is not a struct", value))
github.com/getkin/kin-openapi/openapi3/path_item.go:90:         panic(fmt.Errorf("Unsupported HTTP method '%s'", method))
github.com/getkin/kin-openapi/openapi3/path_item.go:115:                panic(fmt.Errorf("Unsupported HTTP method '%s'", method))
github.com/getkin/kin-openapi/openapi3/schema.go:1198:                  panic(err)
github.com/getkin/kin-openapi/openapi3/schema.go:1202:                  panic(err)
github.com/getkin/kin-openapi/openapi3/schema_formats.go:19:            panic(err)
github.com/getkin/kin-openapi/openapi3filter/req_resp_decoder.go:741:// The function panics when a JSON schema has a non primitive type.
github.com/getkin/kin-openapi/openapi3filter/req_resp_decoder.go:768:           panic(fmt.Sprintf("schema has non primitive type %q", schema.Value.Type))
github.com/getkin/kin-openapi/openapi3filter/req_resp_decoder.go:789:           panic("contentType is empty")
github.com/getkin/kin-openapi/openapi3filter/req_resp_decoder.go:792:           panic("decoder is not defined")
github.com/getkin/kin-openapi/openapi3filter/req_resp_decoder.go:802:           panic("contentType is empty")
github.com/getkin/kin-openapi/openapi3filter/router.go:71:// Panics on any error.
github.com/getkin/kin-openapi/openapi3filter/router.go:74:              panic(err)
github.com/getkin/kin-openapi/openapi3filter/router.go:80:// Panics on any error.
github.com/getkin/kin-openapi/openapi3filter/router.go:83:              panic(err)
github.com/getkin/kin-openapi/pathpattern/node.go:159:          panic(err)
@fenollp
Copy link
Collaborator

fenollp commented Sep 4, 2020

As you may gather from most of these matches the use of panic() is for clear dev error/misuses. Matches that diverge from this should be fixed with a pull request.

I am rewriting the router code (see #210) so this particular surprise should disappear soon(ish).

Note: if you're writing a JSON-Schema <-> OpenAPI JSON schema two-way converter, I'd encourage you to submit it as a PR to this project (cc #230) :)

@mgirouard
Copy link
Contributor Author

Thanks for following up so quickly. I'll dig more into these and see where I can help.

This was more of a question to understand the "why" behind the panics so I'll close for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants