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

Library should not panic #7

Closed
parsnips opened this issue Apr 1, 2019 · 0 comments
Closed

Library should not panic #7

parsnips opened this issue Apr 1, 2019 · 0 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@parsnips
Copy link
Contributor

parsnips commented Apr 1, 2019

The library uses panic in cases where it can return an error. Suggest in cases where multiple errors be returned to return the first error.

SynapseGo/request.go

Lines 56 to 58 in 9d3ed7f

if len(errs) > 0 {
panic(errs)
}

SynapseGo/request.go

Lines 84 to 86 in 9d3ed7f

if len(errs) > 0 {
panic(errs)
}

SynapseGo/request.go

Lines 112 to 114 in 9d3ed7f

if len(errs) > 0 {
panic(errs)
}

SynapseGo/request.go

Lines 133 to 135 in 9d3ed7f

if len(errs) > 0 {
panic(errs)
}

In this case I'd return an error here... Even better suggestion is to make the Response just be a []byte so that clients can marshal into their own structs, and get rid of readStream entirely.

SynapseGo/read.go

Lines 11 to 14 in 7a3e060

// if data is an empty stream this will cause an unmarshal error
if err != nil {
panic(err)
}

@nitharios nitharios added the enhancement New feature or request label Apr 1, 2019
@nitharios nitharios self-assigned this Apr 1, 2019
@nitharios nitharios added the good first issue Good for newcomers label Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants