Skip to content

Hotfix/file upload pooling #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smarcet merged 3 commits into from
Apr 21, 2026
Merged

Hotfix/file upload pooling #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7739e0e
fix(dropzona): add file pooling status
smarcet Apr 21, 2026
15c561c
v5.0.11-beta.1
smarcet Apr 21, 2026
346111a
Merge branch 'main' into hotfix/file-upload-pooling
smarcet Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "openstack-uicore-foundation",
"version": "5.0.10-beta.4",
"version": "5.0.11-beta.1",
"description": "ui reactjs components for openstack marketing site",
"main": "lib/openstack-uicore-foundation.js",
"scripts": {
Expand Down
47 changes: 47 additions & 0 deletions src/components/inputs/dropzone/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,42 @@ export class DropzoneJS extends React.Component {
this.props.onUploadComplete(response, this.props.id, this.props.data);
}

pollUploadStatus(fileId, baseUrl) {
const statusUrl = `${baseUrl}/status/${fileId}`;
const maxAttempts = 300; // 10 minutes at 2s intervals
let attempts = 0;

this._pollInterval = setInterval(async () => {
attempts++;
if (attempts > maxAttempts) {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError({ message: 'Upload timed out' });
return;
}
try {
const accessToken = await getAccessToken();
const response = await fetch(statusUrl, {
headers: { 'Authorization': `Bearer ${accessToken}` }
});
const data = await response.json();
if (data.status === 'complete') {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onUploadComplete(data);
} else if (data.status === 'error') {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError(data);
}
} catch (error) {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError(error);
}
}, 2000);
}
Comment on lines +34 to +68

@coderabbitai coderabbitai Bot Apr 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for callers configuring parallelUploads / multi-file usage of DropzoneJS
rg -nP -C3 '\b(DropzoneJS|djsConfig|parallelUploads|maxFiles|uploadMultiple)\b'

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 23442

🏁 Script executed:

# Search for where pollUploadStatus is called
rg -nP 'pollUploadStatus' src/

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 271

🏁 Script executed:

# Search for HTTP 202 or async upload handling in the dropzone component
rg -nP '202|async.*upload' src/components/inputs/dropzone/

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 173

🏁 Script executed:

# Look for how the dropzone initializes defaults and what parallelUploads setting is used
rg -nP 'parallelUploads|Dropzone\(' src/components/inputs/dropzone/ -A 3 -B 3

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 1208

🏁 Script executed:

# Get context around line 326 where pollUploadStatus is called
sed -n '310,340p' src/components/inputs/dropzone/index.js

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 1265

🏁 Script executed:

# Get the getDjsConfig method
sed -n '70,130p' src/components/inputs/dropzone/index.js

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 2284

Polling mechanism has several robustness gaps that will bite in production.

  1. Concurrent uploads clobber this._pollInterval. Dropzone.js defaults to parallelUploads: 2, allowing multiple files to upload in parallel. When both files receive HTTP 202 responses, both call pollUploadStatus, overwriting this._pollInterval. The first file's interval is orphaned and keeps running, calling onUploadComplete/onError spuriously. Only the most recent interval is cleared on unmount. Use a map (this._pollIntervals[fileId]) to track one interval per file and clean all on unmount.

  2. Any transient failure aborts polling. A single network blip, a 5xx error, or a non-JSON response body (causing response.json() to throw) immediately clears the interval and fires onError. For a 10‑minute async upload, transient failures are almost guaranteed. Treat request failures as retriable; only give up after maxAttempts or an explicit status === 'error'.

  3. No response.ok check. A 401/403 (e.g., token expired mid-poll) or 500 with a JSON body is currently parsed as a normal status payload. Check response.ok before parsing and treat non-2xx responses as transient failures.

  4. setInterval + async overlaps. If getAccessToken() + fetch takes >2s (plausible under load), the next tick fires before the previous completes, issuing overlapping token requests and status calls. Use self-scheduling setTimeout recursion so the next poll is scheduled only after the previous one resolves.

♻️ Sketch of a safer polling loop 
-    pollUploadStatus(fileId, baseUrl) {
-        const statusUrl = `${baseUrl}/status/${fileId}`;
-        const maxAttempts = 300; // 10 minutes at 2s intervals
-        let attempts = 0;
-
-        this._pollInterval = setInterval(async () => {
-            attempts++;
-            if (attempts > maxAttempts) {
-                clearInterval(this._pollInterval);
-                this._pollInterval = null;
-                this.onError({ message: 'Upload timed out' });
-                return;
-            }
-            try {
-                const accessToken = await getAccessToken();
-                const response = await fetch(statusUrl, {
-                    headers: { 'Authorization': `Bearer ${accessToken}` }
-                });
-                const data = await response.json();
-                if (data.status === 'complete') {
-                    clearInterval(this._pollInterval);
-                    this._pollInterval = null;
-                    this.onUploadComplete(data);
-                } else if (data.status === 'error') {
-                    clearInterval(this._pollInterval);
-                    this._pollInterval = null;
-                    this.onError(data);
-                }
-            } catch (error) {
-                clearInterval(this._pollInterval);
-                this._pollInterval = null;
-                this.onError(error);
-            }
-        }, 2000);
-    }
+    pollUploadStatus(fileId, baseUrl) {
+        if (!fileId) {
+            this.onError({ message: 'Missing file_id for async upload' });
+            return;
+        }
+        const statusUrl = `${baseUrl}/status/${fileId}`;
+        const maxAttempts = 300; // 10 minutes at 2s intervals
+        let attempts = 0;
+        this._pollTimers = this._pollTimers || new Map();
+
+        const stop = () => {
+            const t = this._pollTimers.get(fileId);
+            if (t) { clearTimeout(t); this._pollTimers.delete(fileId); }
+        };
+
+        const tick = async () => {
+            if (++attempts > maxAttempts) {
+                stop();
+                this.onError({ message: 'Upload timed out' });
+                return;
+            }
+            try {
+                const accessToken = await getAccessToken();
+                const response = await fetch(statusUrl, {
+                    headers: { 'Authorization': `Bearer ${accessToken}` }
+                });
+                if (response.ok) {
+                    const data = await response.json();
+                    if (data.status === 'complete') { stop(); this.onUploadComplete(data); return; }
+                    if (data.status === 'error')    { stop(); this.onError(data);         return; }
+                }
+                // transient: fall through to reschedule
+            } catch (e) {
+                // transient: log and reschedule
+                console.warn('DropzoneJS::pollUploadStatus transient error', e);
+            }
+            this._pollTimers.set(fileId, setTimeout(tick, 2000));
+        };
+
+        this._pollTimers.set(fileId, setTimeout(tick, 2000));
+    }

And update componentWillUnmount:

-        if (this._pollInterval) {
-            clearInterval(this._pollInterval);
-            this._pollInterval = null;
-        }
+        if (this._pollTimers) {
+            this._pollTimers.forEach((t) => clearTimeout(t));
+            this._pollTimers.clear();
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pollUploadStatus(fileId, baseUrl) {
const statusUrl = `${baseUrl}/status/${fileId}`;
const maxAttempts = 300; // 10 minutes at 2s intervals
let attempts = 0;
this._pollInterval = setInterval(async () => {
attempts++;
if (attempts > maxAttempts) {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError({ message: 'Upload timed out' });
return;
}
try {
const accessToken = await getAccessToken();
const response = await fetch(statusUrl, {
headers: { 'Authorization': `Bearer ${accessToken}` }
});
const data = await response.json();
if (data.status === 'complete') {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onUploadComplete(data);
} else if (data.status === 'error') {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError(data);
}
} catch (error) {
clearInterval(this._pollInterval);
this._pollInterval = null;
this.onError(error);
}
}, 2000);
}
pollUploadStatus(fileId, baseUrl) {
if (!fileId) {
this.onError({ message: 'Missing file_id for async upload' });
return;
}
const statusUrl = `${baseUrl}/status/${fileId}`;
const maxAttempts = 300; // 10 minutes at 2s intervals
let attempts = 0;
this._pollTimers = this._pollTimers || new Map();
const stop = () => {
const t = this._pollTimers.get(fileId);
if (t) { clearTimeout(t); this._pollTimers.delete(fileId); }
};
const tick = async () => {
if (++attempts > maxAttempts) {
stop();
this.onError({ message: 'Upload timed out' });
return;
}
try {
const accessToken = await getAccessToken();
const response = await fetch(statusUrl, {
headers: { 'Authorization': `Bearer ${accessToken}` }
});
if (response.ok) {
const data = await response.json();
if (data.status === 'complete') { stop(); this.onUploadComplete(data); return; }
if (data.status === 'error') { stop(); this.onError(data); return; }
}
// transient: fall through to reschedule
} catch (e) {
// transient: log and reschedule
console.warn('DropzoneJS::pollUploadStatus transient error', e);
}
this._pollTimers.set(fileId, setTimeout(tick, 2000));
};
this._pollTimers.set(fileId, setTimeout(tick, 2000));
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 34 - 68, The current
pollUploadStatus implementation overwrites a single this._pollInterval, aborts
polling on any transient error, fails to check response.ok, and uses setInterval
with async work causing overlapping calls; fix by switching to a per-file map
(e.g., this._pollIntervals[fileId]) and a self-scheduling loop (use setTimeout
recursion) inside pollUploadStatus that checks response.ok before parsing,
treats non‑ok or fetch/json errors as retriable (only stop on maxAttempts or
when data.status === 'error' or 'complete'), and clears and deletes the specific
this._pollIntervals[fileId] when finished; also update componentWillUnmount to
iterate this._pollIntervals and clear all timers, and keep using
onUploadComplete/onError and getAccessToken as before.

@santipalenque santipalenque Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarcet this seems a valid argument: Concurrent uploads clobber this._pollInterval. Dropzone.js defaults to parallelUploads: 2, allowing multiple files to upload in parallel. When both files receive HTTP 202 responses, both call pollUploadStatus, overwriting this._pollInterval. The first file's interval is orphaned and keeps running, calling onUploadComplete/onError spuriously. Only the most recent interval is cleared on unmount. Use a map (this._pollIntervals[fileId]) to track one interval per file and clean all on unmount.

@coderabbitai coderabbitai Bot Apr 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


/**
* Configuration of Dropzone.js. Defaults are
* overriden by the 'djsConfig' property
Expand Down Expand Up @@ -106,6 +142,10 @@ export class DropzoneJS extends React.Component {
* Removes dropzone.js (and all its globals) if the component is being unmounted
*/
componentWillUnmount () {
if (this._pollInterval) {
clearInterval(this._pollInterval);
this._pollInterval = null;
}
if (this.dropzone) {
const files = this.dropzone.getActiveFiles();

Expand Down Expand Up @@ -278,6 +318,13 @@ export class DropzoneJS extends React.Component {
_this.onUploadComplete(uploadResponse);
}
}
else if(xhr?.status == 202) {
// Async upload: server accepted the file, poll for completion
let uploadResponse = JSON.parse(xhr.responseText);
const fileId = uploadResponse.file_id;
const baseUrl = _this.props.config.postUrl;
_this.pollUploadStatus(fileId, baseUrl);
}
Comment on lines +321 to +327

@coderabbitai coderabbitai Bot Apr 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate the 202 payload before starting the poll.

  • JSON.parse(xhr.responseText) can throw on an empty/non-JSON body; the exception would propagate out of xhr.onload and no onError would be reported to the consumer. Wrap in try/catch and call onError on failure.
  • No guard for a missing file_id. If the server returns 202 without file_id, polling will hit ${baseUrl}/status/undefined for 10 minutes before timing out.
  • baseUrl = this.props.config.postUrl is concatenated as ${baseUrl}/status/${fileId}. If postUrl ever contains a query string or trailing slash, the resulting URL will be malformed. Consider using a dedicated statusUrl / statusPath config field rather than deriving it from postUrl.
🛡️ Proposed guard 
-                else if(xhr?.status == 202) {
-                    // Async upload: server accepted the file, poll for completion
-                    let uploadResponse = JSON.parse(xhr.responseText);
-                    const fileId = uploadResponse.file_id;
-                    const baseUrl = _this.props.config.postUrl;
-                    _this.pollUploadStatus(fileId, baseUrl);
-                }
+                else if(xhr?.status == 202) {
+                    // Async upload: server accepted the file, poll for completion
+                    let uploadResponse;
+                    try {
+                        uploadResponse = JSON.parse(xhr.responseText);
+                    } catch (parseErr) {
+                        _this.onError(parseErr, xhr?.status);
+                        return;
+                    }
+                    const fileId = uploadResponse?.file_id;
+                    if (!fileId) {
+                        _this.onError({ message: 'Async upload accepted (202) but response is missing file_id' }, xhr?.status);
+                        return;
+                    }
+                    _this.pollUploadStatus(fileId, _this.props.config.postUrl);
+                }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
else if(xhr?.status == 202) {
// Async upload: server accepted the file, poll for completion
let uploadResponse = JSON.parse(xhr.responseText);
const fileId = uploadResponse.file_id;
const baseUrl = _this.props.config.postUrl;
_this.pollUploadStatus(fileId, baseUrl);
}
else if(xhr?.status == 202) {
// Async upload: server accepted the file, poll for completion
let uploadResponse;
try {
uploadResponse = JSON.parse(xhr.responseText);
} catch (parseErr) {
_this.onError(parseErr, xhr?.status);
return;
}
const fileId = uploadResponse?.file_id;
if (!fileId) {
_this.onError({ message: 'Async upload accepted (202) but response is missing file_id' }, xhr?.status);
return;
}
_this.pollUploadStatus(fileId, _this.props.config.postUrl);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 321 - 327, Wrap the 202
handling in a try/catch when calling JSON.parse(xhr.responseText) and call the
component's onError callback on parse failure; after parsing, validate that
uploadResponse.file_id exists and call onError if missing instead of starting
the poll; avoid constructing the status endpoint by appending to
this.props.config.postUrl — either read a dedicated statusUrl/statusPath config
or sanitize postUrl (remove trailing slash and strip query string) before
calling _this.pollUploadStatus(fileId, baseUrl); update the branch that
currently references xhr, uploadResponse.file_id, _this.pollUploadStatus and
this.props.config.postUrl to perform these checks and error reports before
initiating polling.

else{
_this.onError(JSON.parse(xhr?.responseText), xhr?.status);
}
Expand Down
Loading