Priority field should be optional according to API spec

[auhlig]

Managing an incident should not require the priority field according to the API spec. It's marked as optional.
However, trying to manage an incident without this field returns:

err="Failed call API endpoint. HTTP response code: 400. Error: &{2001 Invalid Input Provided [Priority id cannot be empty.]}"

Moreover, the incident priority feature is disabled in our Pagerduty.
Can you confirm that's a bug? Kindly asking for help here @mattstratton 🙏

[atomicules]

I don't think this is a bug in the API, but a bug introduced in this go-pagerduty library. I say that because it's possible to roll back and use an earlier version of this library and have things work.

I have not tracked down exactly what is causing it, but I suspect related to this commit:

• c94ddd7

So... via Go modules I can have this in go.mod to use an earlier version:

module stuff
require (
        github.com/google/go-querystring v1.0.0
        github.com/pagerduty/go-pagerduty v0.0.0-20180528123509-b4a4067bdbde
)

And then (difficult to communicate this via snippets of code, but...) I have some code which we use for batch resolving alerts:

type incident struct {
	id, summary string
}
[...]
incidents := make([]incident, 0)
[...]
inctoresolve := pagerduty.Incident{
	APIObject: pagerduty.APIObject{
		ID:   incidents[i].id,
		Type: "incident_reference",
	},
	Status: "resolved",
}
[...]
batchIncs = append(batchIncs, incstoresolve)
for b := range batchIncs {
	if err := client.ManageIncidents(*email, batchIncs[b]); err != nil {
		log.Println(err)
	} else {
		log.Println("Resolving incidents...")
	}
}

And this works fine.

But if I use a recent version of this library:

module stuff

require (
        github.com/google/go-querystring v1.0.0
        github.com/pagerduty/go-pagerduty v0.0.0-20190820225956-768e5bce92a6
)

then I hit the same [Priority id cannot be empty.] issue. If I tweak the code so it's like this:

inctoresolve := pagerduty.Incident{
	APIObject: pagerduty.APIObject{
		ID:   incidents[i].id,
		Type: "incident_reference",
	},
	Priority: pagerduty.Priority{
		APIObject: pagerduty.APIObject{
			ID:   "blah",
			Type: "priority_reference",
		},
	},
	Status: "resolved",

Then I get rid of all the priority warnings but end up getting a 400 ("Caller provided invalid arguments") from the API.

I thought that might have been because I don't actually have a Priority ID of blah, but it also doesn't work if I use one of the Priority IDs we actually have (We do actually have Priorities on our account, but not in the context of the incidents I'm working with) and neither does it work if I try to set Priority to nil.

So I think what happens is that this library now inadvertently sets a Priority field on an incident (whether you add explicitly add one into your code or not) and then the api will see that and complain it's not properly formed (Priority id cannot be empty) and then if you try to complete the structure of that to get past those warnings you then run into issues with other parts of the API which aren't expecting to see Priorities. Maybe? So maybe it is kind of a bug in the API in a way.

I could be wrong. Anyway... just leaving that in case it helps anyone else figure it out. Will continue digging.

[atomicules]

I don't understand whyomitempty isn't doing the trick here?

[atomicules]

Ended up doing this to client.go:

func (c *Client) put(path string, payload interface{}, headers *map[string]string) (*http.Response, error) {

	if payload != nil {
		data, err := json.Marshal(payload)
		fmt.Println(string(data))
[...]

So I could see what gets sent:

{"incidents":[{"type":"incident_reference","service":{},"last_status_change_by":{},"first_trigger_log_entry":{},"escalation_policy":{},"priority":{},"status":"resolved","id":"PXXXXX","resolve_reason":{"incident":{}},"alert_counts":{},"body":{}}]}

I'm really not the best with Go, but seems like it's something like this: https://stackoverflow.com/questions/33447334/golang-json-marshal-how-to-omit-empty-nested-struct

[atomicules]

For my use case, changing incident.go to this:

	Priority             *Priority          `json:"priority,omitempty"`

Works. But unsure if PRing just that is enough. Might do it though, at least for discussion.

[atomicules]

PR ^ fwiw.

[auhlig]

Thanks for digging into this @atomicules ❤️ Using a pointer should fix this

[auhlig]

This affects also other field: sapcc@304f7ea

[stmcallister]

Appears #164 addressed this. Closing Issue. Please reopen if this is still a problem.