Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-17327] HTTPClient ondatastream returns progress outside of 0.0 - 1.0

GitHub Issuen/a
TypeBug
PriorityMedium
StatusClosed
ResolutionFixed
Resolution Date2014-12-05T19:01:17.000+0000
Affected Version/sRelease 3.2.3
Fix Version/sRelease 3.5.0, Release 4.0.0
ComponentsiOS
Labelsn/a
ReporterReuben Turk
AssigneeHieu Pham
Created2014-06-13T00:35:47.000+0000
Updated2014-12-11T19:17:50.000+0000

Description

The docs for the HTTPClient ondatastream callback state: {quote} Function to be called at regular intervals as the request data is being received. Must be set before calling open. The progress property of the event will contain a value from 0.0-1.0 with the progress of the request. {quote} In my app the progress number returned is not always between zero and one. Some example values I have received are: * 0.4423390328884125 * 0.9344342350959778 * 1 * 1.599303126335144 * 1.453825831413269 * 1.2088645696640015 * 1.066718339920044 * 2.387096881866455 * 345 (this one is pretty wild) So some are in the range, some are not, and that one 345 is crazy. I'm not sure if it matters but I am running a lot of requests at once (could they possibly interfere with each other?). A few of them are requests to a website that return a bunch of JSON data. The rest are requests for image files from the same web server. There are about 40 - 50 requests total. The requests are using a username and password to get through http authentication. If I run the same set of requests multiple times (within one run of the app) in a row they return exactly the same progress numbers each time. If I stop and start the app again I get different progress numbers. Although the 345 number is always there. The 345 number is for a request for around 100kb of JSON. The numbers mid way between 1 & 2 (and 345) are the JSON requests (although some of these requests return 1). The numbers in the 2s seem to all be image requests where the request has failed. All other images seem to give correct (or almost correct results), being between 0 and about 1.01. So it would seem that the JSON requests and the erroring requests are the ones that return incorrect progress. And maybe the docs should mention rounding when they say 1.0, because a see a lot between 1.0 and 1.01. I can provide more information if required. I will see if I can get a simple code example that exhibits this same problem.

Comments

  1. Ingo Muschenetz 2014-08-11

    Can we confirm if this happens the same way for 3.3.0.GA?
  2. Tim Poulsen 2014-11-19

    Android native accepts a starting value (0) and max value (e.g. 100) and returns an integer value between those ranges representing the amount completed. If developers stick to 0 and 100, that integer value corresponds to a percent complete. Our SDK on both Android and iOS returns a float value between 0 and 1. I've tested this using the KitchenSink app, SDK 3.4.1.GA, to confirm that the values match our documentation. Both platforms natively perform a simple division to determine the percent complete, using the total bytes to send/receive and the bytes already sent/received. If our SDK or the client application sends an incorrect value for the total bytes, then the resulting progress value will likewise be off. For example, if the SDK/client app specifies that 100 KB as the total, yet the actual size is greater, once more than 100KB is sent, e.progress would report values greater than 1.
  3. Ingo Muschenetz 2014-11-19

    Would it be fair (for now) to just place a floor of 0 and a ceiling of 1? Even if the calculation is still off, at least it would be within the documented range and not cause ancillary effects?
  4. Tim Poulsen 2014-11-19

    Ingo, I think that would be an okay short term fix. I think the problem stems from payloads of small or indeterminate sizes. A JSON payload might be small enough that basic block sizes being sent would be larger than the actual payload, leading to values like 345. Perhaps our SDK assumes a default payload size if none is specified / determinable? If that guesstimate is too small, it would also lead to overly-large progress values. We should make sure to Math.abs() so we don't get negative values.
  5. Hieu Pham 2014-11-20

    Android + iOS master PR: https://github.com/appcelerator/titanium_mobile/pull/6364 PR adds ceiling for both ondatastream and onsendstream progress. Doesn't look like progress will ever be below 0, so a floor seem unnecessary.
  6. Pedro Enrique 2014-11-20

    Hey Tim, The crash you are seeing is not about this bug. Looks like you're passing "null" when a "string" was expected. According to the module's source code, your JS code looks somewhat like this 
       var Crittercism = require(‘com.appcelertor.apm);
   Crittercism.init ( "string is supposed to be here" );
    but you're not passing a string, instead you're passing "null" (according to the error message). Make sure that the string is valid and it is what you expect it to be.
  7. Jon Alter 2014-12-03

    We are now setting the progress to 1 if it is below 0. This is how it worked up until TiSDK 3.3.0. Making this change to address MOD-1774 PRs = Hieu's + progress floor of 0 (will now work like before APSHTTPClient aka pre 3.3.0) master: https://github.com/appcelerator/titanium_mobile/pull/6424 3_5_X (I'm guessing you will want this fix here): https://github.com/appcelerator/titanium_mobile/pull/6425
  8. Jon Alter 2014-12-03

    Vishal has an idea of a better way for us to handle the situation where we can't figure out the percentage. The plan is for us to return -1 for any value that does not fall between 0-1 instead of sending a false 1. This way the developer will be able to tell when they are not getting a real value because we can't calculate it. We will have a constant for -1 as well. This will be done for both iOS and Android. If you have any issue with this, please comment. [~ingo] [~skypanther] [~hpham]
  9. Tim Poulsen 2014-12-04

    My concern with returning -1 is that most apps I've seen use e.progress to either output a percentage complete or to use as input to our progress bar component. By returning -1, these apps will output -100% complete, which will cause confusion. I'm not sure how our progress bar component would handle receiving a -1 value when it expects a 0-1 range. Furthermore, developers might not have control over the server to fix an issue where it's reporting invalid sizes that are the underlying cause of this issue. Would it be better to return 0? There'd be no odd output or the progress bar would simply not move. I guess they wouldn't be made specifically aware that there's an issue with their server.
  10. Jon Alter 2014-12-04

    [~skypanther], The plan is that you will be able to check the value before setting the progress indicator value as in the example below. 
        onsendstream: function(e) {
        if (e.progress == Ti.Network.PROGRESS_UNKNOWN) {
            // Do something special if you like
        } else {
            progressBar.value = e.progress;
        }
    }
    If you do not do this check for whatever reason, the progress bar will treat the -1 as a 0 on both iOS and Android. Test code below. 
        var win = Ti.UI.createWindow({
        backgroundColor : 'white',
        layout: 'vertical'
    });
    win.open();
    
    var pb=Titanium.UI.createProgressBar({
        top:30,
        width:250,
        height:'auto',
        min:0,
        max:1,
        value:0
    });
    win.add(pb);
    pb.show();
    
    addButton({
        title: '-1',
        callback: function(e) {
            pb.value = -1;
        }
    });
    
    addButton({
        title: '0',
        callback: function(e) {
            pb.value = 0;
        }
    });
    
    addButton({
        title: '0.5',
        callback: function(e) {
            pb.value = 0.5;
        }
    });
    
    addButton({
        title: '1',
        callback: function(e) {
            pb.value = 1;
        }
    });
    
    function addButton(params) {
        var button = Ti.UI.createButton({
            top: 40,
            title: params.title
        });
        button.addEventListener('click', function() {
            params.callback();
        });
        win.add(button);
    }
  11. Tim Poulsen 2014-12-04

    Cool, returning -1 makes total sense then. Thanks for the clarification.
  12. Jon Alter 2014-12-04

    Updated PRs to return -1 for invalid progress master: https://github.com/appcelerator/titanium_mobile/pull/6424 3_5_X: https://github.com/appcelerator/titanium_mobile/pull/6425
  13. Wilson Luu 2014-12-11

    Closing ticket as fixed. Verified that the fix converts any progress value outside the range of 0.0 - 1.0 to -1. And, also verified Ti.Network.PROGRESS_UNKNOWN returns -1. Tested on: Appcelerator Studio, build: 3.4.1.201410281743 SDK build: 3.5.0.v20141211093314 CLI: 3.4.1 Alloy: 1.5.1 Xcode: 6.2 beta Devices: iPad Air (8.1)

JSON Source