intercom / intercom-go

Go Bindings For Intercom
https://developers.intercom.io/reference
Other
71 stars 80 forks source link

Fix Nil Pointer Crash #17

Closed panamafrancis closed 9 years ago

panamafrancis commented 9 years ago

I came across a really serious bug which caused a exception to be thrown. But my biggest issue is a design one.

I've hacked in the http.Client as a parameter to the intercom client creation it's necessary if you intend to make HTTP calls on Appengine or for any other use case where you need to define a custom HTTP transport. perhaps keep it like that or provide a workaround in the readme.

Perhaps rename HttpClient to ServiceClient or something else as i found that really confusing.

ty.

josler commented 9 years ago

Thanks for submitting the fix!

On the matter of custom HTTP clients however, the library already provides a mechanism for this:

https://godoc.org/github.com/intercom/intercom-go#hdr-HTTP_Client

If you wanted to re-use the existing intercom HTTP client with different options for transport etc:

appEngineClient := http.Client{}
baseURI := "https://api.intercom.io"
clientVersion := "0.0.1"
debug := false
hc := interfaces.IntercomHTTPClient{Client: &appEngineClient, AppID: "APPID", APIKey: "APIKEY", BaseURI: &baseURI, ClientVersion: &clientVersion, Debug: &debug}
ic.Option(intercom.SetHTTPClient(hc))
panamafrancis commented 9 years ago

ah right you are! ok i've reverted my hack then. cheers

josler commented 9 years ago

Thanks :) could we get the commits squashed and I'll merge this in?

Cheers

panamafrancis commented 9 years ago

done :)

josler commented 9 years ago

Thanks!