Fix: Fail gracefully when previewing a doc with deleted reps #962
Conversation
|
Verified that @jeremypress has signed the CLA. Thanks for the pull request! |
jeremypress
changed the title
| // Display a generic error message but log the real one | ||
| const error = new PreviewError(ERROR_CODE.CONTENT_DOWNLOAD, __('error_document'), {}, err.message); | ||
| const error = | ||
| status === 202 |
There was a problem hiding this comment.
Does pdf.js recognize 202 as an error code or is this placeholder?
There was a problem hiding this comment.
it's recognized as a success, but then PDF.js will throw a MissingPDFException, which includes the HTTP status code that led to the error.
src/lib/viewers/doc/DocBaseViewer.js
Outdated
| ERROR_CODE.DELETED_REPS, | ||
| __('error_refresh'), | ||
| { isRepDeleted: true }, | ||
| err.message |
There was a problem hiding this comment.
Can we destructure message, as well?
| url: '**/internal_files/**', | ||
| status: 202, | ||
| response: {} | ||
| }).as('bar'); |
There was a problem hiding this comment.
Are these aliases used elsewhere?
|
|
||
| describe('Document Viewers', () => { | ||
| it('Should correctly handle a document with deleted reps', () => { | ||
| helpers.checkDeletedRepError(); |
There was a problem hiding this comment.
How does this guarantee that the assertion on error happens?
There was a problem hiding this comment.
if this assertion fails, an uncaught exception will be thrown and the test will time out.
|
|
||
| }); | ||
|
|
||
| it('Should correctly handle a presentation with deleted reps', () => { |
There was a problem hiding this comment.
Consider using leche or forEach to cut down on duplication between file types.
| it('Should correctly handle a presentation with deleted reps', () => { | ||
| helpers.checkDeletedRepError(); | ||
|
|
||
| cy.showPreview(token, presentationIdDoc, { showAnnotations: false }); |
There was a problem hiding this comment.
Can you add a comment as to why annotations is disabled.
todo