Javascript do-while loop condition causing error

41 views Asked by At

I am running an older version of Ionic v1 which is no longer supported. It appears I have discovered a bug that is not covered anywhere. The only two mentions of it are 1) related to collection-repeat, which I am not running anywhere in my app and 2) my own previous question(s) about trying to resolve the error.

The bug, reported from my production app, has only happened about a dozen times out of thousands of sessions - but I am not able to replicate it or accurately determine when/where/how its being triggered. The culprit function is anchorScroll which I think is being triggered during a scroll in a modal using an ng-repeat on a list of 160 items. The only reason I believe its related to this is because the only other time curElm.attributes is mentioned in a google search is related to an Ionic collection-repeat and, when combined with the fact that the most items I ever had in this ng-repeat was 20 items, I believe the 160 list is a part of the scrolling culprit here. But I don't know for certain.

This is from Ionc v1 ionic.bundle.js, line 60723

  self.anchorScroll = function(shouldAnimate) {
    self.resize().then(function() {
      if (!scrollView) {
        return;
      }
      var hash = $location.hash();
      var elm = hash && $document[0].getElementById(hash);
      if (!(hash && elm)) {
        scrollView.scrollTo(0, 0, !!shouldAnimate);
        return;
      }
      var curElm = elm;
      var scrollLeft = 0, scrollTop = 0;
      do {
        if (curElm !== null) scrollLeft += curElm.offsetLeft;
        if (curElm !== null) scrollTop += curElm.offsetTop;
        curElm = curElm.offsetParent;
      } while (curElm.attributes != self.element.attributes && curElm.offsetParent);  // ERROR HERE
      scrollView.scrollTo(scrollLeft, scrollTop, !!shouldAnimate);
    });
  };

What is the best way to patch the above function so it doesn't trigger the following error?

Exception = null is not an object (evaluating 'curElm.attributes')
Stack = @ionic://attendago/lib/ionic/js/ionic.bundle.js:60723:22
processQueue@ionic://attendago/lib/ionic/js/ionic.bundle.js:29132:30
@ionic://attendago/lib/ionic/js/ionic.bundle.js:29148:39
completeOutstandingRequest@ionic://attendago/lib/ionic/js/ionic.bundle.js:19199:15
@ionic://attendago/lib/ionic/js/ionic.bundle.js:19475:33

Would it be to simply rewrite the while statement to be as follows:

} while (curElm && curElm.attributes != self.element.attributes && curElm.offsetParent);

Or would it be to test for null during the curElm assisgnment:

if (curElm !== null) curElm = curElm.offsetParent;

...or something else? Because I haven't been able to replicate the problem I am not able to really test my own patch so I need to be extra careful that my own patch doesn't cause more problems.

All the rest of the trace seems related to defer'd process queues, adding them in case it helps: 1st @ line: 19475

  self.defer = function(fn, delay) {
    var timeoutId;
    outstandingRequestCount++;
    timeoutId = setTimeout(function() {
      delete pendingDeferIds[timeoutId]; 
      completeOutstandingRequest(fn);   //19475 ***
    }, delay || 0);
    pendingDeferIds[timeoutId] = true;
    return timeoutId;
  };

2nd @ line: 19199

  function completeOutstandingRequest(fn) {
    try {
      fn.apply(null, sliceArgs(arguments, 1));  //19199 ***
    } finally {
      outstandingRequestCount--;
      if (outstandingRequestCount === 0) {
        while (outstandingRequestCallbacks.length) {
          try {
            outstandingRequestCallbacks.pop()();
          } catch (e) {
            $log.error(e);
          }
        }
      }
    }
  }

3rd @ line: 29148

  function scheduleProcessQueue(state) {
    if (state.processScheduled || !state.pending) return;
    state.processScheduled = true;
    nextTick(function() { processQueue(state); });  //29148 ***
  }

4th @ line: 29132

  function processQueue(state) {
    var fn, deferred, pending;

    pending = state.pending;
    state.processScheduled = false;
    state.pending = undefined;
    for (var i = 0, ii = pending.length; i < ii; ++i) {
      deferred = pending[i][0];
      fn = pending[i][state.status];
      try {
        if (isFunction(fn)) {
          deferred.resolve(fn(state.value));  //29132 ***
        } else if (state.status === 1) {
          deferred.resolve(state.value);
        } else {
          deferred.reject(state.value);
        }
      } catch (e) {
        deferred.reject(e);
        exceptionHandler(e);
      }
    }
  }
2

There are 2 answers

0
This Guy On

like you mention, this is tough to figure out when you can't replicate the issue.

Your logs / error state the issue is that curElm is 'null', so you should try to avoid that / test for that condition.

I would think that var curElm = elm; might be causing the issue. you could try to use 'let' rather than var. My worry is that because of the var declaration, another function might be re-assigning this variable from another code block.

The other place is

curElm = curElm.offsetParent; 

I think modifying the while condition could be beneficial :

while ( curElm !== null && curElm.attributes != self.element.attributes && curElm.offsetParent);  // ERROR HERE

I hope this helps :)

0
rolinger On

For anyone who comes across this issue, this is how I dealt with it.

In ionic.budle.js, I changed line 60723 from:

while ( curElm.attributes != self.element.attributes && curElm.offsetParent);  

to

while ( curElm && curElm.attributes != self.element.attributes && curElm.offsetParent);  

NOTE: I don't fully know what impact this has on performance or scrolling; but at least its not generating an error anymore.