nightscout / cgm-remote-monitor

nightscout web monitor
GNU Affero General Public License v3.0
2.38k stars 71.59k forks source link

APIv3 wrong result #8096

Closed MilosKozak closed 11 months ago

MilosKozak commented 11 months ago

Sometimes happen this:

D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: --> GET https://XXXXXXXXX.ns.10be.de/api/v3/treatments/history/1694958729909?limit=500 h2
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: Date: 1694959031997
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: Authorization: Bearer eyJhbGciOiJI.....
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: Host: XXXXXXXXXXXXX.ns.10be.de
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: Connection: Keep-Alive
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: Accept-Encoding: gzip
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: User-Agent: okhttp/4.11.0
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: --> END GET
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: <-- 200 https://XXXXXXXXX.ns.10be.de/api/v3/treatments/history/1694958729909?limit=500 (72ms)
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: server: nginx
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: date: Sun, 17 Sep 2023 13:57:10 GMT
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: content-type: application/json; charset=utf-8
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: content-length: 476
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: access-control-allow-origin: *
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: access-control-allow-methods: GET,PUT,POST,DELETE,OPTIONS
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: access-control-allow-headers: Content-Type, Authorization, Content-Length, X-Requested-With
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: last-modified: Sunday, 17-Sep-2023 13:57:10 GMT
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: etag: W/"1694958729909"
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: vary: Accept, Accept-Encoding
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: cache-control: no-store, no-cache, must-revalidate, proxy-revalidate, max-age=0
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: 
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: {"status":200,"result":[{"absolute":0.3,"app":"AAPS","date":1694958731440,"duration":30,"durationInMilliseconds":1800000,"eventType":"Temp Basal","isReadOnly":false,"isValid":true,"pumpId":1694958731440,"pumpSerial":"do9-DU5eT26XcMRlyZ6LId","pumpType":"OMNIPOD_DASH","rate":0.3,"type":"NORMAL","utcOffset":120,"created_at":"2023-09-17T13:52:11.440Z","identifier":"9a800246-3f95-51aa-a874-92510944a697","srvModified":1694958729909,"srvCreated":1694958729909,"subject":"aaps"}]}
D/HTTP: [NSClientV3Plugin.setClient$lambda$6():338]: <-- END HTTP (476-byte body)

ie querying records with srvModified > 1694958729909 but as you see record with srvModified==1694958729909 is returned

MilosKozak commented 11 months ago

nevermind i probably found it: record's date > srvModified but question is if it's expected behavior. current implementation in AAPS is that if I get non empty result I continue quering (expecting there could be more records) with the timestamp of etag: W/"1694958729909" which holds largest srvModified in returned set. In this case it leads to endless loop

MilosKozak commented 11 months ago

Thinking of the logic it should be based only on srvModified https://github.com/nightscout/cgm-remote-monitor/blob/master/lib/api3/generic/history/operation.js#L106

MilosKozak commented 11 months ago

... and using srvModified which is based on server's clock and timestamps in records (based on client's clock) in one query can lead to other potential errors

bewest commented 11 months ago

Should it be possible to use created_at instead? Does everything through /api/v1 get srvModified? I don't think that happens, although it could.

MilosKozak commented 11 months ago

srvModified nad srvCreated is populated only through v3 maybe that's the reason why date and created_at is here https://github.com/nightscout/cgm-remote-monitor/blob/master/lib/api3/generic/history/operation.js#L106 but generally it breaks the logic of sync with v3 I'd recommend keep only srvModified . I declared in release notes to use only v3 for upload once you turn it on in AAPS I hope v1 will be removed or adapted in the future

MilosKozak commented 11 months ago

https://github.com/nightscout/cgm-remote-monitor/pull/8100 with this patch sync stuck no more

bjornoleh commented 11 months ago

With #8100 merged, I guess this issue can be closed.