fix: send right response for heartbeat.bulk

[v1ack]

Implement heartbeat response

[muety]

Looks good! Just for the record, this is where response is parsed in the CLI and this is what the response model is supposed to look like.

But still, could you please briefly explain why this change is needed / what problem it solves?

[v1ack]

I wanted to use browser extension with wakapi and faced some issues:

• nil in first position in heartbeat response
• wrong id
• 201 responce code - Consider 201 status code as success wakatime/browser-wakatime#284 - looks it's also needs to be fixed on wakapi side

This change based on wakatime docs, I didn't opened cli source code.

[v1ack]

Also macos wakatime app doesn't work)
But as for now I can't blame wakapi, I couldn't capture any requests to my instance, except getting work time. And there isn't anything interesting in the logs on macos app side.
But one day I'll get to it, too)

[muety]

Thanks for the update. The Wakatime plugin is being a bit overly strict there. Generally, every status between 200 and 299 should be considered a success. But looks like we'll have to adapt then, if they're not willing to.

I'm having a super busy time right now. Will do the review some time this week! Please bear with me...

Also, keep me posted on the macOS issue and let me know if you get it to work.

[v1ack]

Thanks!
Macos app works fine, there was a permission issue on my side.

The last problem I have is <<LAST_PROJECT>> and <<LAST_LANGUAGE>> in the dashboard.
Maybe it's better to open an issue, and I'll look deeper into it on the weekend

[muety]

Can you describe what's the exact bug with <<LAST_PROJECT>> and <<LAST_LANGUAGE>> placeholders?

[v1ack]

Placeholders appear in the dashboard and it looks a bit confusing

[muety]

I'd assume this is for data sent by the browser plugin, correct?

Wakapi intentionally ignored <<LAST_PROJECT>> (maps it to unknown), because I didn't think the intention behind that placeholder makes much sense. See discussion here. The <<LAST_LANGUAGE>> field is currently not recognized properly, which is a bug. Created #687 for this.

[alanhamlett]

The Wakatime plugin is being a bit overly strict there. Generally, every status between 200 and 299 should be considered a success.

For context, we're strict with the status code because some corporate proxies or misconfigured networks have been known to return 200 status, but the request never made it to the WakaTime API servers. This way we store heartbeats in your local offline db until the situation is fixed instead of thinking they were received and throwing away those code stats.

Edit: However, that status code was one nested in the response body array not the actual HTTP response status so we can modify the browser extension to support any 2XX nested status as successful.

[alanhamlett]

https://github.com/wakatime/browser-wakatime/releases/tag/4.0.7

[v1ack]

@muety

I'd assume this is for data sent by the browser plugin, correct?

No, macOS app, sent with type = app

Example of row in my db

17304,vlack,Skype,app,coding,<<LAST_PROJECT>>,"","",false,Macos,Darwin,MacBook-Pro-Vladimir.local,wakatime/v1.102.1 (darwin-23.3.0-arm64) go1.22.5 SkypeforBusiness/16.30.32-16.30.32 macos-wakatime/5.24.0,2024-10-09 13:37:28.873,351e2eeb669b16b9,"","",2024-10-09 13:37:29.104

[muety]

Finally got to look into your changes. Unfortunately, API tests are failing, as you changed the serialization format of CustomTime.

  #  failure                                         detail                                                                                                                                                                                    
                                                                                                                                                                                                                                               
 1.  AssertionError                                  Correct dates                                                                                                                                                                             
                                                     expected NaN to be at least 1728770400                                                                                                                                                    
                                                     at assertion:3 in test-script                                                                                                                                                             
                                                     inside "Summary / Get summary (range)"                                                                                                                                                    
                                                                                                                                                                                                                                               
 2.  AssertionError                                  Correct time zone                                                                                                                                                                         
                                                     expected false to deeply equal true                                                                                                                                                       
                                                     at assertion:1 in test-script                                                                                                                                                             
                                                     inside "Summary / Get summary (default tz)"                                                                                                                                               
                                                                                                                                                                                                                                               
 3.  AssertionError                                  Correct time zone                                                                                                                                                                         
                                                     expected false to deeply equal true                                                                                                                                                       
                                                     at assertion:1 in test-script                                                                                                                                                             
                                                     inside "Summary / Get summary (parse tz)"

Can you please try to fix them?

Maybe wait until we got a reply from Alan here - preferably we don't even need any changes on our end.

Relates to #688.