node-strava / node-strava-v3

API wrapper for Strava's v3 API, in Node
MIT License
356 stars 109 forks source link

Detailed segment efforts have id field with the last few digits zeroed, for efforts which are recent / have very long id fields #125

Open IanPeake opened 2 years ago

IanPeake commented 2 years ago

When retrieving detailed segment efforts, for recent efforts with very long id fields, the last few digits of the id field are zeroes.

This seems to be reliably reproducible---will aim to provide a test case.

Not sure if it is strava or node-strava-v3 doing this.

IanPeake commented 2 years ago

Test case:

const efforts = await strava.segments.listEfforts({id: "30903832",per_page: 100}); console.log("Nr efforts found >= ",efforts.length); for (e of efforts) { try { console.log("Effort id "+e.id); const e2 = await strava.segmentEfforts.get({id: e.id}); console.log(e2); } catch (err) { console.log("main: Error:", typeof(err), err.message); } }

markstos commented 2 years ago

Please test some direct API calls to confirm if this what the Strava API is returning or something we are doing wrong.

IanPeake commented 2 years ago

I have added test code in the comments which demonstrates this.

On Thu, 17 Feb. 2022, 03:12 Mark Stosberg, @.***> wrote:

Please test some direct API calls to confirm if this what the Strava API is returning or something we are doing wrong.

— Reply to this email directly, view it on GitHub https://github.com/node-strava/node-strava-v3/issues/125#issuecomment-1041831265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFSEU6O36ZQXJRJZMG7K2FLU3PEGRANCNFSM5OROM5PQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

IanPeake commented 2 years ago

Sorry, misread your comment. Not sure yet how to distinguish what API does vs node-strava yet. Will get back to you.

On Thu, 17 Feb. 2022, 07:03 Ian Peake, @.***> wrote:

I have added test code in the comments which demonstrates this.

On Thu, 17 Feb. 2022, 03:12 Mark Stosberg, @.***> wrote:

Please test some direct API calls to confirm if this what the Strava API is returning or something we are doing wrong.

— Reply to this email directly, view it on GitHub https://github.com/node-strava/node-strava-v3/issues/125#issuecomment-1041831265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFSEU6O36ZQXJRJZMG7K2FLU3PEGRANCNFSM5OROM5PQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

IanPeake commented 2 years ago

Here's a test (using the above code) demonstrating that cURL is giving efforts with correct IDs, whereas strava-v3 isn't:

$ ./test_eff_id.js Strava token init Strava API check... response: [object Object] Nr efforts found >= 3 Effort id 2927332094085175000 main: Error: object 404 - {"message":"Record Not Found","errors":[]} Effort id 2922283800928553000 main: Error: object 404 - {"message":"Record Not Found","errors":[]} Effort id 2924806434787802600 main: Error: object 404 - {"message":"Record Not Found","errors":[]}

$ curl -H "Authorization: Bearer d5f3aa919e06766ec9dbd5bb257261758409e524" 'https://www.strava.com/api/v3/segments/30903832/all_efforts?per_page=100' [{"id":2927332094085174610,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity":{"id":6668026999,"resource_state":1},"athlete":{"id":21463851,"resource_state":1},"elapsed_time":1592,"moving_time":1585,"start_date":"2022-02-11T21:01:38Z","start_date_local":"2022-02-12T08:01:38Z","distance":5074.4,"start_index":1,"end_index":372,"average_cadence":86.8,"device_watts":false,"average_heartrate":172.1,"max_heartrate":185.0,"segment":{"id":30903832,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity_type":"Run","distance":5074.4,"average_grade":0.0,"maximum_grade":5.1,"elevation_high":88.4,"elevation_low":77.0,"start_latlng":[-37.867567,145.248895],"end_latlng":[-37.866796,145.249216],"elevation_profile":null,"start_latitude":-37.867567,"start_longitude":145.248895,"end_latitude":-37.866796,"end_longitude":145.249216,"climb_category":0,"city":"Wantirna South","state":"Victoria","country":"Australia","private":false,"hazardous":false,"starred":true},"pr_rank":1,"achievements":[{"type_id":3,"type":"pr","rank":1}],"kom_rank":null},{"id":2922283800928553202,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity":{"id":6560086394,"resource_state":1},"athlete":{"id":21463851,"resource_state":1},"elapsed_time":1603,"moving_time":1602,"start_date":"2022-01-21T20:59:12Z","start_date_local":"2022-01-22T07:59:12Z","distance":5074.4,"start_index":0,"end_index":370,"average_cadence":88.0,"device_watts":false,"average_heartrate":176.2,"max_heartrate":186.0,"segment":{"id":30903832,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity_type":"Run","distance":5074.4,"average_grade":0.0,"maximum_grade":5.1,"elevation_high":88.4,"elevation_low":77.0,"start_latlng":[-37.867567,145.248895],"end_latlng":[-37.866796,145.249216],"elevation_profile":null,"start_latitude":-37.867567,"start_longitude":145.248895,"end_latitude":-37.866796,"end_longitude":145.249216,"climb_category":0,"city":"Wantirna South","state":"Victoria","country":"Australia","private":false,"hazardous":false,"starred":true},"pr_rank":null,"achievements":[],"kom_rank":null},{"id":2924806434787802582,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity":{"id":6631650714,"resource_state":1},"athlete":{"id":21463851,"resource_state":1},"elapsed_time":1648,"moving_time":1648,"start_date":"2022-02-04T20:59:57Z","start_date_local":"2022-02-05T07:59:57Z","distance":5074.4,"start_index":2,"end_index":383,"average_cadence":85.2,"device_watts":false,"average_heartrate":168.6,"max_heartrate":184.0,"segment":{"id":30903832,"resource_state":2,"name":"Lewis Park Reserve parkrun","activity_type":"Run","distance":5074.4,"average_grade":0.0,"maximum_grade":5.1,"elevation_high":88.4,"elevation_low":77.0,"start_latlng":[-37.867567,145.248895],"end_latlng":[-37.866796,145.249216],"elevation_profile":null,"start_latitude":-37.867567,"start_longitude":145.248895,"end_latitude":-37.866796,"end_longitude":145.249216,"climb_category":0,"city":"Wantirna South","state":"Victoria","country":"Australia","private":false,"hazardous":false,"starred":true},"pr_rank":2,"achievements":[{"type_id":3,"type":"pr","rank":2}],"kom_rank":null}]

IanPeake commented 2 years ago

Is there a simple way to trace the response being received via strava-v3? Setting NODE_DEBUG='request' seems to trace only the request part and not the response.

IanPeake commented 2 years ago

The culprit appears to be the JSON parser built into request-promise / request?

If I disable json:true in the request header I get

response... [{"id":2927332094085174610,"resource_state":2,"name":"Lewis Park Reserv e parkrun","activity":{"id":6668026999,"resource_state":1},"athlete":{"id":21463851 ,"resource_state":1},"elapsed_time":1592,"moving_time":1585,"start_date":"2022-02-1 1T21:01:38Z","start_date_local":"2022-02-12T08:01:38Z","distance":5074.4,"start_ind ex":1,"end_index":372,"average_cadence":86.8,"device_watts":false,"average_heartrat e":172.1,"max_heartrate":185.0,"segment":{"id":30903832,"resource_state":2,"name":" Lewis Park Reserve parkrun","activity_type":"Run","distance":5074.4,"average_grade" :0.0,"maximum_grade":5.1,"elevation_high":88.4,"elevation_low":77.0,"start_latlng": [-37.867567,145.248895],"end_latlng":[-37.866796,145.249216],"elevation_profile":nu ll,"start_latitude":-37.867567,"start_longitude":145.248895,"end_latitude":-37.8667 96,"end_longitude":145.249216,"climb_category":0,"city":"Wantirna South","state":"V ictoria","country":"Australia","private":false,"hazardous":false,"starred":true},"p r_rank":1,"achievements":[{"type_id":3,"type":"pr","rank":1}],"kom_rank":null},{"id ":2922283800928553202,"resource_state":2,"name":"Lewis Park Reserve parkrun","activ ity":{"id":6560086394,"resource_state":1},"athlete":{"id":21463851,"resource_state" :1},"elapsed_time":1603,"moving_time":1602,"start_date":"2022-01-21T20:59:12Z","sta rt_date_local":"2022-01-22T07:59:12Z","distance":5074.4,"start_index":0,"end_index" :370,"average_cadence":88.0,"device_watts":false,"average_heartrate":176.2,"max_hea rtrate":186.0,"segment":{"id":30903832,"resource_state":2,"name":"Lewis Park Reserv e parkrun","activity_type":"Run","distance":5074.4,"average_grade":0.0,"maximum_gra de":5.1,"elevation_high":88.4,"elevation_low":77.0,"start_latlng":[-37.867567,145.2 48895],"end_latlng":[-37.866796,145.249216],"elevation_profile":null,"start_latitud e":-37.867567,"start_longitude":145.248895,"end_latitude":-37.866796,"end_longitude ":145.249216,"climb_category":0,"city":"Wantirna South","state":"Victoria","country

markstos commented 2 years ago

Could you test with node-fetch? request and request-promise are deprecated and due to be replaced in our code base anyway. Something like this:

//  Requires Node 16 and a file extension of ".mjs"
import fetch from 'node-fetch';
const response = await fetch('https://www.strava.com/api/v3/segments/30903832/all_efforts?per_page=100', {
    method: 'get',
    headers: {
    "Authorization": "Bearer YOUR_TOKEN_HERE",
    'Content-Type': 'application/json'
  }
});
const data = await response.json();
console.log(data);
markstos commented 2 years ago

I this can be considered a bug on Strava's side. They are returning numbers that exceed JavaScript's MAX_SAFE_INTEGER value of "9007199254740991". A recommended fix for the Strava JSON to represent these as strings, not numbers. Clients can safely handle these values as strings. I'll reach out to Strava.

2922283800928553000  <- What node-strava-v3 returns for a segment effort ID
2922283800928553202  <- What a direct API call with Curl returns for the same
9007199254740991        <- For reference, the max Int size in Javascript            
markstos commented 2 years ago

I posted in the public forum here: https://groups.google.com/g/strava-api/c/dgY870wgTIo

IanPeake commented 2 years ago

Thanks @markstos !

markstos commented 2 years ago

Official response from Strava:

The team is aware of this issue and has a ticket in the backlog for handling. While we can't speak to an expected resolution date, we'll be sure to update the forums once it's been made.

They don't say explicitly that they are going to encode these values as strings, but by saying they are going to handle it on their side, it's the logical and most likely option.

So we have a choice now to try to apply our own "forward-compatible" fix until Strava address this upstream.

The JSON we receive from Strava is just a string, a text document. We parse that text and rewrite it to quote the IDs as strings ourselves and THEN run it through a JSON parser. Because this problem affects the entire JavaScript ecosystem, I expect there is likely already example code and an NPM package to help with this.

I'm unlikely to work on this, so feel free to pick up this up if you are interested.

IanPeake commented 2 years ago

Great, thanks for the update.

On Wed, 23 Feb. 2022, 08:48 Mark Stosberg, @.***> wrote:

Official response from Strava:

The team is aware of this issue and has a ticket in the backlog for handling. While we can't speak to an expected resolution date, we'll be sure to update the forums once it's been made.

They don't say explicitly that they are going to encode these values as strings, but by saying they are going to handle it on their side, it's the logical and most likely option.

So we have a choice now to try to apply our own "forward-compatible" fix until Strava address this upstream.

The JSON we receive from Strava is just a string, a text document. We parse that text and rewrite it to quote the IDs as strings ourselves and THEN run it through a JSON parser. Because this problem affects the entire JavaScript ecosystem, I expect there is likely already example code and an NPM package to help with this.

I'm unlikely to work on this, so feel free to pick up this up if you are interested.

— Reply to this email directly, view it on GitHub https://github.com/node-strava/node-strava-v3/issues/125#issuecomment-1048244212, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFSEU6LWAOWOMBGHNX5RD7TU4QABDANCNFSM5OROM5PQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

markstos commented 2 years ago

Here's example code from the Ruby client for testing to see if truncation is happening:

https://github.com/dblock/strava-ruby-client/pull/51/files