Hi and thanks a lot for this great module!

Context:

The default timeout in dropzone is 30.000 ms (see http://www.dropzonejs.com/#config-timeout) which means 30 seconds. This is much too short for large files or slow internet connections. And especially for such large files dropzonejs is very interesting to indicate the progress.

Dropzone Version: 5.4.0

Problem:

  • Large files can not be uploaded or people with a slow internet connection can not even upload smaller files
  • The upload gets stuck with no error message. The progress bar just stops and nothing happens. No JS error, nothing. The POST is just aborted.

Proposed solution:

a) Make the timeout configurable via a setting
b) Might be enough (and is quick): Override the dropzone default with a higher value like 300 seconds and make it alterable via hook for more special cases

BONUS: Display an error message if the timeout is reached.

I'll attach a patch for (b) firstofall to fix the major problem. Of course we can then discuss to implement (a) additionally.

CommentFileSizeAuthor
#21 dropzonejs-upload-timeout--2977463-21.patch1.85 KBjcnventura
#17 dropzonejs-upload-timeout--2977463-17.patch1.73 KBjungle
#10 dropzonejs-upload-timeout--2977463-10.patch1.74 KBesolitos
#4 dropzonejs_8.x-2.x-increase_timeout-2977463-2.patch553 bytesanybody
#2 dropzonejs_8.x-1.x-increase_timeout-2977463-2.patch554 bytesanybody

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 8.x-2.x-dev » 8.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new dropzonejs_8.x-1.x-increase_timeout-2977463-2.patch554 bytes

8.x-1.x version patch attached.
8.x-2.x version patch will follow (if it's not identical)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
StatusFileSize
new dropzonejs_8.x-2.x-increase_timeout-2977463-2.patch553 bytes

8.x-2.x version patch attached.

The last submitted patch, 2: dropzonejs_8.x-1.x-increase_timeout-2977463-2.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 4: dropzonejs_8.x-2.x-increase_timeout-2977463-2.patch, failed testing. View results

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Timeout much too short for large files and no error message » Default timeout much too short for large files / slow connections and no error message
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Well the failing tests seem unrelated?! I guess the test file upload doesn't work here? Can the maintainer explain what's wrong perhaps?

We've tested both patches manually with success.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review
esolitos’s picture

esolitos
he/him
Primary language Italian
Location Trondheim
commented
StatusFileSize
new dropzonejs-upload-timeout--2977463-10.patch1.74 KB

Hi, I have updated the patch so it's a configurable value in milliseconds: upload_timeout_ms , right now there's no form so it needs to be set manually, which is the same as the current other existing config tmp_upload_scheme.

esolitos’s picture

esolitos
he/him
Primary language Italian
Location Trondheim
commented
Title: Default timeout much too short for large files / slow connections and no error message » Allow configuration of dropzonejs timeout, default value causes silent errors.
Priority: Normal » Major

Change of priority because the default behaviour it causes silent failures on client side with no logging possible and there's no workaround.

Status: Needs review » Needs work

The last submitted patch, 10: dropzonejs-upload-timeout--2977463-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

esolitos’s picture

esolitos
he/him
Primary language Italian
Location Trondheim
commented
Status: Needs work » Needs review

Errors on tests seems unrelated.

Status: Needs review » Needs work

The last submitted patch, 10: dropzonejs-upload-timeout--2977463-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

primsi’s picture

primsi commented
Issue tags: +drupalmountaincamp
szeidler’s picture

szeidler commented
Status: Needs work » Needs review

I can confirm, that patch #10 is working fine. It is quite major bug, when considering having authors, that have a slow connection (e.g. mobile) and a rather large file size. The default 30 seconds from the library are unreasonable small.

The test failure seems to be unrelated to this patch.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
StatusFileSize
new dropzonejs-upload-timeout--2977463-17.patch1.73 KB

how about setting the default timeout to 0? Which should mean no timeout but haven't found from its documentation.

keboca’s picture

keboca commented

FYI guys I applied the patch #17 and it solved my problems! it works as expected! thx!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Also confirming RTBC of #17, thank you! Any active maintainer ready for the final review and commit?

wturrell’s picture

wturrell commented

Thank you for the patch. Also works nicely for me (core 8.8.4, dropzonejs 8.x-2.0 and dropzone 5.7.0)

(If anyone has done any theming work to make it look a bit nicer / cleaner / more Bartik compatible, I'd be interested…)

jcnventura’s picture

jcnventura
Primary language English
commented
StatusFileSize
new dropzonejs-upload-timeout--2977463-21.patch1.85 KB

Reroll of #17 for dropzone 8.x-2.1

No interdiff as I couldn't generate one, but I've only changed the dropzonejs_update_8003() function to follow the same code pattern as 8001 and 8002.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Thank you @jcnventura for working on this.

+++ b/src/Element/DropzoneJs.php
@@ -138,6 +138,7 @@ class DropzoneJs extends FormElement {
+          'timeout' => \Drupal::configFactory()->get('dropzonejs.settings')->get('upload_timeout_ms'),

\Drupal usage should be avoided, use IoC injection instead.

jcnventura’s picture

jcnventura
Primary language English
commented

@jungle I only re-rolled your patch, as it was blocking our deployments.

This particular DropzoneJs.php file has 5 other invocations of \Drupal. One of them for the exact same config.factory service. The dependecy injection should be solved in a separate issue. If that issue exists, maybe this one can be marked as blocked by it.

Otherwise, I don't think this issue is the place to solve the lack of proper DI code in this module.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Agree with you that to do it in a separate issue, thanks for your reply,

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Re-confirming RTBC for #21! Thank you!

Hopefully we'll see this in the next release :)

  • jungle committed 550caaa on 8.x-2.x authored by Anybody 
    Issue #2977463 by Anybody, jungle, esolitos, jcnventura: Allow...
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks everyone for working on this!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.