I just had a little goose-chase with Go's panic(). If that happens inside a REST API handler, apparently there is no way to notice. The response is just returned empty. Turns out that protocol.DeviceIDFromBytes() called panic() because of a wrong slice length.

So what is the preferred method of handling such errors? I tried this in the model's handling function, but didn't succeed:

defer recover()
db.ListPendingDevices(m.db)   // panicked in here!

recover is fickle: The panic must occur in the same goroutine. So my guess is something inside db.ListPendingDevices creates a goroutine which then panics.

You don't handle panics in go, you fix them. As in they should not be part of normal operations. I don't yet have an idea what might be going on in your case, as I didn't review this yet (will do). If you need help to continue, please give some pointers at what to look.

And the teamcity log is completely messed up. If I grep the full build log I find the relevant info within completely irrelevant lines:

--- PASS: TestIgnoredFiles (0.08s)
[20:59:23]i:	 [Step 4/5] {"Time":"2020-03-22T20:59:23.955164703Z","Action":"output","Package":"github.com/syncthing/syncthing/lib/db","Test":"TestIgnoredFiles","Output":"--- PASS: TestIgnoredFiles (0.08s)\n"}
[20:59:23]i:	 [Step 4/5] {"Time":"2020-03-22T20:59:23.955728758Z","Action":"pass","Package":"github.com/syncthing/syncthing/lib/db","Test":"TestIgnoredFiles","Elapsed":0.08}
[20:59:23]i:		 [github.com/syncthing/syncthing/lib/db] TestUpdate0to3
[20:59:23]i:			 [TestUpdate0to3] gotest://github.com/syncthing/syncthing/lib/db#TestUpdate0to3
[20:59:23]i:	 [Step 4/5] {"Time":"2020-03-22T20:59:23.956022633Z","Action":"run","Package":"github.com/syncthing/syncthing/lib/db","Test":"TestUpdate0to3"}
[20:59:23]i:			 [TestUpdate0to3] [Test Output]
=== RUN   TestUpdate0to3
[20:59:23]i:	 [Step 4/5] {"Time":"2020-03-22T20:59:23.956284174Z","Action":"output","Package":"github.com/syncthing/syncthing/lib/db","Test":"TestUpdate0to3","Output":"=== RUN   TestUpdate0to3\n"}
[20:59:23]W:	 [Step 4/5] # github.com/syncthing/syncthing/lib/api [github.com/syncthing/syncthing/lib/api.test]
[20:59:23]W:	 [Step 4/5] lib/api/api_test.go:528:21: cannot use m (type *mockedModel) as type model.Model in argument to ur.New:
[20:59:23]W:	 [Step 4/5] 	*mockedModel does not implement model.Model (missing PendingDevices method)
[20:59:23]W:	 [Step 4/5] lib/api/api_test.go:529:52: cannot use m (type *mockedModel) as type model.Model in argument to New:
[20:59:23]W:	 [Step 4/5] 	*mockedModel does not implement model.Model (missing PendingDevices method)
[20:59:23]i:			 [TestUpdate0to3] [Test Output]
20:59:23 INFO: File{Name:"a", Sequence:1, Permissions:00, ModTime:1970-01-01 00:00:00 +0000 UTC, Version:{[{AAAAAAA 1000}]}, Length:0, Deleted:false, Invalid:false, LocalFlags:0x0, NoPermissions:false, BlockSize:0, Blocks:[Block{0/0/0/000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f}]}
[20:59:23]i:	 [Step 4/5] {"Time":"2020-03-22T20:59:23.988160815Z","Action":"output","Package":"github.com/syncthing/syncthing/lib/db","Test":"TestUpdate0to3","Output":"20:59:23 INFO: File{Name:\"a\", Sequence:1, Permissions:00, ModTime:1970-01-01 00:00:00 +0000 UTC, Version:{[{AAAAAAA 1000}]}, Length:0, Deleted:false, Invalid:false, LocalFlags:0x0, NoPermissions:false, BlockSize:0, Blocks:[Block{0/0/0/000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f}]}\n"}
[20:59:23]i:			 [TestUpdate0to3] [Test Output]
--- PASS: TestUpdate0to3 (0.03s)

@calmh @AudriusButkevicius Any clue what might be going on here?

The cannot use mockedmodel ... stuff is a compile error... But yeah I can't defend the output there, other than maybe as one package being compiled concurrently with another running tests?

Also, not really essential as the panic (whatever it is) shouldn't be recovered, but defer recover() will never work. recover() needs to be called from a deferred function.

I actually don't intend to just recover the panic, which does not help anyway. Was just wondering about the fact that it quietly dies without any kind of logging the panic. I'll make sure to check the device ID format before trying to convert it, so it shouldn't panic. Finding the cause for empty responses when testing was unnecessarily hard though, when runtime errors (malformed data in this case) are reported nowhere.

Frankly I haven't even looked at the failing checks as this is still very incomplete. Lacking any test coverage at the least.

What I would like in terms of review is if the code structure and responsibilities as well as introduced data structures are sensible. Or if something sticks out that should definitely be done another way. Otherwise I still have much ground to cover and will keep pushing code bits as they start producing sensible output.

The cannot use mockedmodel ... stuff is a compile error... But yeah I can't defend the output there, other than maybe as one package being compiled concurrently with another running tests?

I get that, but I should be able to see the compile error in TCs interface, shouldn't I? I am unable to do so.

At this point, pending devices are actually stored in the DB and can be retreived through the discussed API endpoint. Happy testing, folks!

Please advise if this is going in the right direction and the code quality / style is acceptable so far. Then I'll move on to duplicate for the pending folders and add more functionality.

Thank you @imsodin :-)