From 7f750ae692834572e5b06f8482ff85c1f627d873 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 12 Jan 2015 16:45:24 +0000 Subject: [PATCH 1/8] add an error callback to the pdf renderer use page load and render timeouts of 30 seconds to detect hanging pages --- .../ide/pdfng/directives/pdfRenderer.coffee | 70 ++++++++++++++++--- .../ide/pdfng/directives/pdfViewer.coffee | 2 + 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee index 15a45c8908..c04af476ea 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee @@ -19,9 +19,11 @@ define [ @navigateFn = @options.navigateFn @spinner = new pdfSpinner @resetState() - + @errorCallback = @options.errorCallback + @pdfjs.catch (exception) => + # console.log 'ERROR in get document', exception + @errorCallback(exception) resetState: () -> - @page = [] @complete = [] @timeout = [] @pageLoad = [] @@ -34,9 +36,8 @@ define [ pdfDocument.numPages getPage: (pageNum) -> - # with promise caching - return @page[pageNum] if @page[pageNum]? - @page[pageNum] = @document.then (pdfDocument) -> + @document.then (pdfDocument) -> + # console.log 'got pdf document, now getting Page', pageNum pdfDocument.getPage(pageNum) getPdfViewport: (pageNum, scale) -> @@ -44,6 +45,8 @@ define [ @document.then (pdfDocument) -> pdfDocument.getPage(pageNum).then (page) -> viewport = page.getViewport scale + , (error) -> + console.log 'ERROR', error getDestinations: () -> @document.then (pdfDocument) -> @@ -57,15 +60,20 @@ define [ pdfDocument.getDestinations() return @destinations.then (all) -> all[dest] + , (error) -> + console.log 'ERROR', error - @document.then (pdfDocument) -> - pdfDocument.getDestination(dest) - + # @document.then (pdfDocument) -> + # pdfDocument.getDestination(dest) + # , (error) -> + # console.log 'ERROR', error getPageIndex: (ref) -> @document.then (pdfDocument) -> pdfDocument.getPageIndex(ref).then (idx) -> idx + , (error) -> + console.log 'ERROR', error getScale: () -> @scale @@ -101,6 +109,7 @@ define [ @triggerRenderQueue() processRenderQueue: () -> + return if @shuttingDown return if @jobs > 0 current = @renderQueue.shift() return unless current? @@ -118,17 +127,37 @@ define [ completeRef = @complete renderTaskRef = @renderTask + # console.log 'started page load', pagenum + + timedOut = false + timer = $timeout () => + # console.log 'page load timed out', pagenum + timedOut = true + @spinner.stop(element.canvas) + # @jobs = @jobs - 1 + # @triggerRenderQueue(0) + this.errorCallback?('timeout') + , 30*1000 @pageLoad[pagenum] = @getPage(pagenum) + @pageLoad[pagenum].then (pageObject) => + # console.log 'in page load success', pagenum + $timeout.cancel(timer) @renderTask[pagenum] = @doRender element, pagenum, pageObject @renderTask[pagenum].then () => # complete + # console.log 'render task success', pagenum completeRef[pagenum] = true @removeCompletedJob renderTaskRef, pagenum , () => + # console.log 'render task failed', pagenum # rejected @removeCompletedJob renderTaskRef, pagenum + .catch (error) -> + # console.log 'in page load error', pagenum, 'timedOut=', timedOut + $timeout.cancel(timer) + # console.log 'ERROR', error doRender: (element, pagenum, page) -> self = this @@ -183,17 +212,42 @@ define [ element.canvas.replaceWith(canvas) + # console.log 'staring page render', pagenum + result = page.render { canvasContext: ctx viewport: viewport } + timedOut = false + + timer = $timeout () -> + # console.log 'page render timed out', pagenum + timedOut = true + result.cancel() + , 30*1000 + result.then () -> + # console.log 'page rendered', pagenum + $timeout.cancel(timer) canvas.removeClass('pdfng-rendering') page.getTextContent().then (textContent) -> textLayer.setTextContent textContent + , (error) -> + console.log 'ERROR', error page.getAnnotations().then (annotations) -> annotationsLayer.setAnnotations annotations + , (error) -> + console.log 'ERROR', error + .catch (error) -> + # console.log 'page render failed', pagenum, error + $timeout.cancel(timer) + if timedOut + # console.log 'calling ERROR callback - was timeout' + self.errorCallback?('timeout') + else if error != 'cancelled' + # console.log 'calling ERROR callback' + self.errorCallback?(error) return result diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index 7f3ac48250..e3d5df0adf 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -35,6 +35,8 @@ define [ $scope.$apply() progressCallback: (progress) -> $scope.$emit 'progress', progress + errorCallback: (error) -> + $scope.$emit 'pdf:error', error }) # we will have all the main information needed to start display From bf8bc27de0997a1fbe9638c94bd981cb3fe378ce Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 12 Jan 2015 16:46:17 +0000 Subject: [PATCH 2/8] catch errors in pdf viewer and reload if necessary if more than 3 reloads, display error to the user --- .../coffee/ide/pdf/controllers/PdfController.coffee | 3 +++ .../coffee/ide/pdfng/directives/pdfViewer.coffee | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 610009bf8c..eae6d8052d 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -10,6 +10,9 @@ define [ $scope.recompile(isAutoCompile: true) $scope.hasPremiumCompile = $scope.project.features.compileGroup == "priority" + $scope.$on "pdf:error:display", () -> + $scope.pdf.error = true + sendCompileRequest = (options = {}) -> url = "/project/#{$scope.project_id}/compile" if options.isAutoCompile diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index e3d5df0adf..8d4100c9ba 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -25,7 +25,7 @@ define [ # $scope.pages = [] $scope.document.destroy() if $scope.document? - + scope.loadCount = scope.loadCount? ? scope.loadCount + 1 : 1 # TODO need a proper url manipulation library to add to query string $scope.document = new PDFRenderer($scope.pdfSrc + '&pdfng=true' , { scale: 1, @@ -293,6 +293,16 @@ define [ ] #scope.$apply() + scope.$on 'pdf:error', (event, error) -> + return if error == 'cancelled' + # check if too many retries or file is missing + if scope.loadCount > 3 || error.match(/^Missing PDF/i) + scope.$emit 'pdf:error:display' + return + ctrl.load() + # trigger a redraw + scope.scale = angular.copy (scope.scale) + element.on 'scroll', () -> #console.log 'scroll event', element.scrollTop(), 'adjusting?', scope.adjustingScroll if scope.adjustingScroll @@ -317,6 +327,7 @@ define [ scope.$watch 'pdfSrc', (newVal, oldVal) -> # console.log 'loading pdf', newVal, oldVal return unless newVal? + scope.loadCount = 0; # new pdf, so reset load count ctrl.load() # trigger a redraw scope.scale = angular.copy (scope.scale) From 560919b78fb7cadb8ed0c0d9d41948ca6f14408b Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 12 Jan 2015 16:47:38 +0000 Subject: [PATCH 3/8] avoid exception in pdf viewer if file was previously bigger, current position could be greater than the number of pages --- .../web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index 8d4100c9ba..aa42b28db9 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -92,6 +92,8 @@ define [ # console.log 'position is', position.page, position.offset # console.log 'setting current page', position.page pagenum = position.page + if pagenum > $scope.numPages - 1 + pagenum = $scope.numPages - 1 $scope.pages[pagenum].current = true $scope.pages[pagenum].position = position From 887423f8d7ea48831f6ca4f5df5a9b943ee871c5 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 12 Jan 2015 17:02:51 +0000 Subject: [PATCH 4/8] report timeout errors to sentry --- .../coffee/ide/pdfng/directives/pdfRenderer.coffee | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee index c04af476ea..8c9dd798ef 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee @@ -7,6 +7,8 @@ define [ class PDFRenderer JOB_QUEUE_INTERVAL: 25 + PAGE_LOAD_TIMEOUT: 30*1000 + PAGE_RENDER_TIMEOUT: 30*1000 constructor: (@url, @options) -> PDFJS.disableFontFace = true # avoids repaints, uses worker more @@ -131,13 +133,14 @@ define [ timedOut = false timer = $timeout () => + Raven.captureMessage?('pdfng page load timed out after ' + @PAGE_LOAD_TIMEOUT + 'ms') # console.log 'page load timed out', pagenum timedOut = true @spinner.stop(element.canvas) # @jobs = @jobs - 1 # @triggerRenderQueue(0) this.errorCallback?('timeout') - , 30*1000 + , @PAGE_LOAD_TIMEOUT @pageLoad[pagenum] = @getPage(pagenum) @@ -221,11 +224,12 @@ define [ timedOut = false - timer = $timeout () -> + timer = $timeout () => + Raven.captureMessage?('pdfng page render timed out after ' + @PAGE_RENDER_TIMEOUT + 'ms') # console.log 'page render timed out', pagenum timedOut = true result.cancel() - , 30*1000 + , @PAGE_RENDER_TIMEOUT result.then () -> # console.log 'page rendered', pagenum From 823bdcf583d29b68d75f2bf7fca644fc6e62c599 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 12 Jan 2015 17:03:03 +0000 Subject: [PATCH 5/8] fix scope error in pdf viewer --- .../web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index aa42b28db9..a94a4a5fda 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -25,7 +25,7 @@ define [ # $scope.pages = [] $scope.document.destroy() if $scope.document? - scope.loadCount = scope.loadCount? ? scope.loadCount + 1 : 1 + $scope.loadCount = $scope.loadCount? ? $scope.loadCount + 1 : 1 # TODO need a proper url manipulation library to add to query string $scope.document = new PDFRenderer($scope.pdfSrc + '&pdfng=true' , { scale: 1, From 1d3d316595234354315fbb3d7fad745ee48f870e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 13 Jan 2015 15:47:27 +0000 Subject: [PATCH 6/8] capture all pdfng error callbacks with raven --- services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index a94a4a5fda..844c572054 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -36,6 +36,7 @@ define [ progressCallback: (progress) -> $scope.$emit 'progress', progress errorCallback: (error) -> + Raven.captureMessage?('pdfng error ' + error) $scope.$emit 'pdf:error', error }) From 8d568d96630c24c0445fbcee754c84d57da7394e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 13 Jan 2015 16:38:40 +0000 Subject: [PATCH 7/8] increase pdfng page load/render timeouts to 1 minute --- .../web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee index 8c9dd798ef..123a850049 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee @@ -7,8 +7,8 @@ define [ class PDFRenderer JOB_QUEUE_INTERVAL: 25 - PAGE_LOAD_TIMEOUT: 30*1000 - PAGE_RENDER_TIMEOUT: 30*1000 + PAGE_LOAD_TIMEOUT: 60*1000 + PAGE_RENDER_TIMEOUT: 60*1000 constructor: (@url, @options) -> PDFJS.disableFontFace = true # avoids repaints, uses worker more From d8324069239c454abc2e82cf261bf52d1561b6be Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 16 Jan 2015 12:50:08 +0000 Subject: [PATCH 8/8] pdfng: add comment about getDestinations vs getDestination --- .../coffee/ide/pdfng/directives/pdfRenderer.coffee | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee index 123a850049..a173986203 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfRenderer.coffee @@ -54,17 +54,19 @@ define [ @document.then (pdfDocument) -> pdfDocument.getDestinations() -# Not available in pdf.js-1.0.712, in later versions there is a direct -# call for this - we should use it as soon as it is available in a -# stable version getDestination: (dest) -> + # There is a direct method for this in pdf.js but it is not + # available in pdf.js-1.0.712. Use the following workaround of + # getting all the destinations and returning only the one we + # want. @destinations = @document.then (pdfDocument) -> pdfDocument.getDestinations() return @destinations.then (all) -> all[dest] , (error) -> console.log 'ERROR', error - + # When we upgrade we can switch to using the following direct + # code. # @document.then (pdfDocument) -> # pdfDocument.getDestination(dest) # , (error) ->