Improvements to site thumbnail generation #1081
Conversation
| const finishedLoading = new Promise< void >( ( resolve ) => { | ||
| window.webContents.on( 'did-finish-load', () => resolve() ); | ||
| } ); |
There was a problem hiding this comment.
window.loadURL returns a promise that resolves when the did-finish-load event has fired, so I took the opportunity to simplify this logic.
| window.webContents.on( 'did-finish-load', () => resolve() ); | ||
| } ); | ||
| const responseStatusCodePromise = new Promise< void >( ( resolve, reject ) => { | ||
| newSession.webRequest.onCompleted( ( details ) => { |
There was a problem hiding this comment.
Adding a URL filter (docs) did not work well in my testing. Likely because of potential redirects.
| .catch( Sentry.captureException ) | ||
| .catch( async ( error ) => { | ||
| Sentry.captureException( error ); | ||
| await fs.promises.unlink( outPath ); |
There was a problem hiding this comment.
Deleting the thumbnail path makes the UI look like this:
There was a problem hiding this comment.
I know it's intentional for the thumbnail to look this way when there's no image. My point was that deleting the thumbnail when the homepage returns an errant HTTP status is new behavior, so I wanted to clarify what the UI will look like in those cases.
There was a problem hiding this comment.
Thanks for clarifying that, makes sense.
wojtekn
left a comment
wojtekn
left a comment
There was a problem hiding this comment.
Works well on Windows now. MacOS still works fine.
| .catch( Sentry.captureException ) | ||
| .catch( async ( error ) => { | ||
| Sentry.captureException( error ); | ||
| await fs.promises.unlink( outPath ); |
* Throw error when thumbnail page responds with error status * Tweaks * Only increase timeout on Windows * Promise
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Related issues
Proposed Changes
waitForCapture.QUESTION: I'm open to suggestions for which behavior to implement when the site home page returns an error. I think generating a thumbnail makes for weird behavior, but maybe it's better to leave the old thumbnail in place, I'm not sure…
Testing Instructions
No preview availablePre-merge Checklist