refactor(uploads): remove statusCallback in favor of maxStatusChecks

Author: wesleyschlenker

README.md

@@ -186,19 +186,25 @@ const comments = await strava.activities.listComments({
 
 ### Uploading files
 
-To upload a file you'll have to pass in the `data_type` as specified in Strava's API reference as well as a string `file` designating the `<filepath>/<filename>`. If you want to get updates on the status of your upload pass in `statusCallback` along with the rest of your `args` - the wrapper will check on the upload once a second until complete.
+To upload a file you'll have to pass in the `data_type` as specified in Strava's API reference as well as a string `file` designating the `<filepath>/<filename>`. By default no status polling is performed—the promise resolves immediately with the initial upload response. To wait until processing is complete, set `maxStatusChecks` (e.g. `300` for ~5 minutes at 1s intervals); the promise will then resolve with the final upload result.
 
 Example usage:
 
 ```js
 const strava = require('strava-v3');
+// Resolve immediately after posting
 const payload = await strava.uploads.post({
+    data_type: 'gpx',
+    file: 'data/your_file.gpx',
+    name: 'Epic times'
+});
+
+// Or wait until processing is complete (polls once per second, up to maxStatusChecks times)
+const result = await strava.uploads.post({
     data_type: 'gpx',
     file: 'data/your_file.gpx',
     name: 'Epic times',
-    statusCallback: (err,payload) => {
-        //do something with your payload
-    }
+    maxStatusChecks: 300
 });
 ```
 

index.d.ts

@@ -1,5 +1,3 @@
-export type CallbackError = Error | { msg: string } | string;
-
 interface BaseArgs {
   access_token?: string;
   responseType?: string;
@@ -50,7 +48,6 @@ export interface UploadRouteArgs {
   external_id?: string;
   access_token?: string;
   maxStatusChecks?: number;
-  statusCallback?: (error: CallbackError | null, response?: Upload) => void;
 }
 
 export interface Upload {

lib/uploads.js

@@ -16,8 +16,7 @@ const { setTimeout } = require('timers/promises')
  * @property {0|1} [commute] - API expects integer; set to 1 to mark as commute.
  * @property {string} [external_id]
  * @property {string} [access_token]
- * @property {(error: import('../index').CallbackError | null, response?: Upload) => void} [statusCallback]
- * @property {number} [maxStatusChecks] - Max polling attempts when using statusCallback (default 300 ≈ 5 min at 1s).
+ * @property {number} [maxStatusChecks] - Default: no status polling; promise resolves immediately with the initial upload response. If set to a positive number, poll until upload is done or this many attempts (e.g. 300 ≈ 5 min at 1s); promise then resolves with the final Upload.
  * @property {Record<string, string>} [formData] - Set by post(); do not pass.
  */
 
@@ -50,7 +49,7 @@ var uploads = function (client) {
 }
 
 /**
- * Uploads a file to create a new activity; optionally polls status via statusCallback until done.
+ * Uploads a file to create a new activity. By default returns immediately with the initial response. If maxStatusChecks > 0, polls until upload is done and resolves with the final Upload.
  * @param {PostUploadArgs} args
  * @returns {Promise<Upload>}
  */
@@ -76,62 +75,49 @@ uploads.prototype.post = async function (args) {
 
   const response = await this.client.postUpload(args)
 
-  // if no statusCallback, just return after posting upload
-  if (typeof args.statusCallback === 'undefined') {
+  const maxChecks = args.maxStatusChecks != null ? args.maxStatusChecks : 0
+  if (maxChecks === 0) {
     return response
   }
 
-  // otherwise, kick off the status checking loop
-  // and return the result of that
-  // the callback will be called on each status check
   /** @type {CheckArgs} */
   var checkArgs = {
     id: response.id,
     access_token: args.access_token || undefined,
-    maxChecks: args.maxStatusChecks != null ? args.maxStatusChecks : 300
+    maxChecks: maxChecks
   }
-  return await self._check(checkArgs, args.statusCallback)
+  return await self._check(checkArgs)
 }
 
 /**
- * Internal: polls upload status and invokes callback until the upload is done or maxChecks is reached.
+ * Internal: polls upload status until the upload is done or maxChecks is reached.
  * @param {CheckArgs} args
- * @param {(error: import('../index').CallbackError | null, response?: Upload) => void} cb
  * @returns {Promise<Upload>}
  */
-uploads.prototype._check = async function (args, cb) {
+uploads.prototype._check = async function (args) {
   var endpoint = 'uploads/' + args.id
   var self = this
   var maxChecks = args.maxChecks != null ? args.maxChecks : 300
   var attempts = 0
 
-  try {
-    while (true) {
-      attempts++
-      const response = /** @type {Upload} */ (await this.client.getEndpoint(endpoint, args))
+  while (true) {
+    attempts++
+    const response = /** @type {Upload} */ (await this.client.getEndpoint(endpoint, args))
 
-      const error = typeof response.error === 'undefined' ? null : response.error
-      cb(error, response)
-
-      if (self._uploadIsDone(response)) {
-        return response
-      }
-
-      if (attempts >= maxChecks) {
-        break
-      }
+    if (self._uploadIsDone(response)) {
+      return response
+    }
 
-      await setTimeout(1000)
+    if (attempts >= maxChecks) {
+      break
     }
-  } catch (err) {
-    cb(/** @type {import('../index').CallbackError} */ (err))
-    throw err
+
+    await setTimeout(1000)
   }
 
   const timeoutError = new Error(
     'Upload status check timed out after ' + maxChecks + ' attempts (still processing)'
   )
-  cb(/** @type {import('../index').CallbackError} */ (timeoutError))
   throw timeoutError
 }
 

test/uploads.js

@@ -68,28 +68,17 @@ describe('uploads_test', function () {
             status: 'Your activity is ready.'
           })
 
-        let statusCallbackCount = 0
-        let finalPayload = null
-
-        await strava.uploads.post({
+        const result = await strava.uploads.post({
           activity_type: 'run',
           sport_type: 'Run',
           data_type: 'gpx',
           name: 'test activity',
           file: gpxFilename,
-          statusCallback: function (err, payload) {
-            assert.strictEqual(err, null)
-            statusCallbackCount++
-
-            if (payload.status === 'Your activity is ready.') {
-              finalPayload = payload
-            }
-          }
+          maxStatusChecks: 2 // we only mock 2 status responses
         })
 
-        assert.strictEqual(statusCallbackCount, 2)
-        assert.strictEqual(finalPayload.activity_id, activityId)
-        assert.strictEqual(finalPayload.status, 'Your activity is ready.')
+        assert.strictEqual(result.activity_id, activityId)
+        assert.strictEqual(result.status, 'Your activity is ready.')
       } finally {
         tmpFile.removeCallback()
       }