PagerDuty/go-pagerduty

Chaining GetService and UpdateService fails when AlertGroupingParameters is omitted

mjlshen opened this issue · 5 comments

service, err := pd.GetService(serviceID, nil)
if err != nil {
    return err
}

service.status = "disabled"

if _, err = pd.UpdateService(*service); err != nil {
    return err
}

Results in the following when the following error when AlertGroupingParameters is unset/omitted by Go:

"error":"HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001): Alert grouping parameters is invalid."

The JSON response from GetService looks like:

"alert_grouping_parameters": {
      "type": null,
      "config": null
    },

but the resulting struct ends up like, which the PagerDuty API doesn't seem to like:

pd.AlertGroupingParameters{
	Type:   "",
	Config: pd.AlertGroupParamsConfig{
		Timeout:   0,
		Aggregate: "",
		Fields:    nil,
	},
}

As a workaround, a check like this works before calling UpdateService

if service.AlertGroupingParameters.Type == "" {
     service.AlertGroupingParameters = nil
}

I'm not quite sure if there's anything we can do here. Because the field is a pointer, it should be nil if the PagerDuty response omits the object. But as you've shown the API isn't omitting the object, even though all fields are null, which feels like a bug on their side.

I help maintain this library in my spare time to benefit the Go community, but am not employed there. This means I don't have any special access to engineering, so I'm curious if you're able to file a bug report with PagerDuty support?

Appreciate the response! I'll see what I can do and report back here

Didn't have much luck with support, but with a closer look I do think it is a bug in this library where it's missing a couple omitempty's, so I made that PR^

@theckman when you get a chance, I would appreciate a review on my proposed fix - thanks!