NotFoundError when update returns no data objects

[dnesting]

When calling UpdateNetwork, I occasionally get a response containing no data elements:

HTTP/1.1 200
...
{
 "meta": {
  "rc": "ok"
 },
 "data": []
}

This results in a &NotFoundError{} being returned at https://github.com/paultyng/go-unifi/blob/main/unifi/network.generated.go#L392.

I suspect what's happening is that my change ends up being a no-op change from the controller's perspective, so it returns an empty data element. In Terraform land, this results in my apply failing even though it did actually apply "correctly". What should happen here? The implied behavior of the update methods seems to be to return a new copy of the object passed, presumably with any added fields from the controller populated or changed. But it doesn't seem like we can actually rely on that behavior. Should the function return an empty &Network{} instance in this case? nil?

Is there any documentation collecting a description of apparent behavior and assumptions like this?

This is potentially one of many bugs/changes I might want to start hacking on. Is the right thing to do to start opening individual issues, pull requests, or do you have a preference for accepting contributions?

[paultyng]

Documents of our observations are a great idea but sadly I haven't done that yet.

I think in this case, since its already a named error, the best bet is to probably just account for this on the provider side, do an errors.As check and handle it (perhaps looking it back up in the client or something to confirm its not actually not found?).

[dnesting]

To be clear, both of these conditions currently result in NotFoundError responses to the Update* methods in the go-unify client:

1. HTTP 404
2. HTTP 200, rc: "ok", data=[]

The controller's behavior appears to be different in these two cases:

1. The resource doesn't exist, so the update was not applied to anything
2. (Assuming I'm interpreting this right), the resource exists, but the update did not contain any changes (and is already "applied")

Are you saying you prefer the second case to continue to return NotFoundError? This seems like it would make it difficult for callers to know what to do with that error.

[MaienM]

I ran into this a while ago on the Device resource, and in that case I was able to track it down to the portOverrides field being omitted. The request that was getting this empty result was not a no-op in this case, and the requested change was not applied, so this really behaved more like a 400 than a 200 or 404.

The solution for that specific case was to remove omitEmpty for that field so it was always included in the request. The PR for this has some more details including requests/responses.