-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run jsdom shutdown on the next tick to avoid errors being thrown #16
base: master
Are you sure you want to change the base?
Conversation
Fixed it to make sure it's not called twice. But still might have some issues I overlooked... |
This one's interesting. I like the fault tolerance it adds. // Provide a tick for any queued jsdom operations to wrap up without error
setTimeout(() => dom.window.close(), 0) Maybe we should make it an opt-in/opt-out option. Alternatively we could also catch all errors after |
So the reason I was getting this error is basically that I was calling
As you observed, the error seems to have occurred because the page was serialized and On your suggestion: I'm not sure how you would be able to catch errors after Also note that without the guard to call Anyway, I'm using this code on Netlify and it seems to have gotten rid of all the silent failures, so I'll let you decide what to do with this. (Netlify doesn't like an error being thrown after the HTML is generated: if it hasn't sent the response back yet, it'll do something weird as in roxiness/routify-starter#96) |
This should be perfectly fine. Any chance there's an async call running after the data is loaded? I'm puzzled by
if (dom.window._document) {
console.log(`[tossr] ${url} Waited for the event "${eventName}", but timed out after ${timeout} ms.`);
resolveHtml()
// guard against a second call
dom.window.removeEventListener(eventName, resolveHtml) //added
} I will probably have to bite the bullet and write some more extensive tests for |
This might be the same problem. Some async code runs after load, fires the Debugging this further seems above my pay grade. Though I'm using these two commits in my deployment so far, and I haven't see any runtime errors or double-calls. I'll check back in here if I notice anything else. |
Much appreciated! @mizzao |
This fixed #14 for me.
However, I think this deserves a bit more thought. It can lead to
resolveHtml()
getting called twice, for example.