node-strava / node-strava-v3

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

`Error: args must include an athlete id` when running `strava.athletes.get` #152

Closed Yuvix25 closed 8 months ago

Yuvix25 commented 8 months ago

When attempting to retrieve an athlete with:

await strava.athletes.get({ athlete_id: id });

I get the following error:

UnhandledPromiseRejectionWarning: Error: args must include an athlete id
    at athletes._listHelper (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\strava-v3\lib\athletes.js:21:11)
    at athletes.get (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\strava-v3\lib\athletes.js:7:15)
    at Request._rp_callbackOrig (C:\Users\yuvro\Documents\mtb-gpx-analyzer\out\main\index.js:38:49)
    at plumbing.callback (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\request-promise-core\lib\plumbing.js:76:39)
    at Request.RP$callback [as _callback] (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\request-promise-core\lib\plumbing.js:46:31)
    at self.callback (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\request\request.js:185:22)
    at Request.emit (node:events:517:28)
    at Request.<anonymous> (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\request\request.js:1154:10)
    at Request.emit (node:events:517:28)
    at IncomingMessage.<anonymous> (C:\Users\yuvro\Documents\mtb-gpx-analyzer\node_modules\request\request.js:1076:12)
    at Object.onceWrapper (node:events:631:28)
    at IncomingMessage.emit (node:events:529:35)
    at endReadableNT (node:internal/streams/readable:1368:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

There appears to be a mismatch between the types file: https://github.com/node-strava/node-strava-v3/blob/5b2d9ad1c102716a7b8131b69e53df914e1c6556/index.d.ts#L152 And the actual code: https://github.com/node-strava/node-strava-v3/blob/5b2d9ad1c102716a7b8131b69e53df914e1c6556/lib/athletes.js#L20

markstos commented 8 months ago

Looks like you are calling the plural of athletes when you intend the singular athlete.

The singular version doesn't take an id at all because it gets the authenticated athlete details.

https://developers.strava.com/docs/reference/#api-Athletes-getStats

If something could be improved about the code or docs, feel free to submit a PR, but since the wrong method name appears to have been called here, I'm closing this.

Yuvix25 commented 8 months ago

image No. I am calling the correct plural athletes.

markstos commented 8 months ago

Please review the API docs.

https://developers.strava.com/docs/reference/

I see no method to get the details of an arbitrary athlete.

Yuvix25 commented 8 months ago

You're right, it doesn't exist. In that case, this line should probably be removed: https://github.com/node-strava/node-strava-v3/blob/5b2d9ad1c102716a7b8131b69e53df914e1c6556/lib/athletes.js#L6 I got confused by its existence (thought this was an actual endpoint), and I'm sure many others also will.

markstos commented 8 months ago

This is a community-maintained project and a PR would be welcome that cleans up any code and docs that are out of sync or confusing. See also #142 and #149.

There's a comment in the docs about passing an id to athletes.get(), but I think the intent was actually to add that comment toactivity.get()`.

The original author is no longer active and there is no one else standing by to fix issues for other people. The module will improve to the extent that users are willing to contribute improvements.

Yuvix25 commented 8 months ago

Yep, a PR is where I'm going with this. Just opened this issue first to make sure I wasn't missing anything (which I was, so good thing I did it before PRing 🙂) Might also fill a few other type annotations while I'm at it, too many anys in the index.d.ts file for my liking 😁

markstos commented 5 months ago

Tightening the types would be welcome as well.