FWeinb / electron-screenshot-service

Take screenshots using electron
MIT License
141 stars 26 forks source link

Electron error popup if service is not closed immediately after screenshot #24

Closed peerbolte closed 8 years ago

peerbolte commented 8 years ago

Whenever I take a screenshot and not run screenshot.close() afterwards, I will get the following electron error in a popup:

Uncaught Exception:
TypeError: Cannot read property 'id' of null
    at Timeout.makeScreenshot (<PATH>/node_modules/electron-screenshot-service/electron-service/node_modules/electron-screenshot-app/index.js:61:42)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)

Is this expected behaviour? If so the following might be a noob question but here we go:

Do I have to close the service after each screenshot (as this prevents the error from showing)? If so, how do I take another screenshot after closing the service?

screenshot.scale(5) I'm on OSX 10.11 electron-screenshot-service: 3.1.3

FWeinb commented 8 years ago

Can you test this with 3.2.0 You should not have .close() it every time. This is definitely not expected behaviour.

peerbolte commented 8 years ago

Just tried it with 3.2.0, no luck :(. Still getting the errors

FWeinb commented 8 years ago

I just tested it with the example/multiple.js and everything is working fine. Can you provide a simple example that fails for you?

peerbolte commented 8 years ago

example/multiple.js runs screenshot.close() after running it? What happens if you omit that?

FWeinb commented 8 years ago

It runs it after all screenshots where made. Promise.all([]) waits till all the promises are fulfilled.

[Edit] If it is omitted the process will never stop.

peerbolte commented 8 years ago

There are three iframes on the page I am trying to capture. When I visit a page without iframes, the error does not occur. Could this be the possible source for the problem?

FWeinb commented 8 years ago

Thanks for the hint. I will try with multiple iframe!

FWeinb commented 8 years ago

I can't reproduce it here. I tried with multiple screenshot of the front page of codepen.io where 6 iframes are loaded each time and everything is working fine.

peerbolte commented 8 years ago

did you omit screenshot.close() ? In my case I want to keep the process open, as I send multiple domains to process over time.

FWeinb commented 8 years ago

Here is the code I am using.

peerbolte commented 8 years ago

gives a 404

FWeinb commented 8 years ago

Sorry, should work now.

peerbolte commented 8 years ago

hi, this code gives me the error. It's not consistent, you might need to run it a couple of times.

It might have something to do with the load of the websites in the iframes? Or does this occur because screenshot.close() is never called?

FWeinb commented 8 years ago

When you run cat node_modules/electron-prebuilt/dist/version in the root of your project dose this print v1.2.6?

peerbolte commented 8 years ago

yes

FWeinb commented 8 years ago

This is a hard one... I can't reproduce it on my side.

peerbolte commented 8 years ago

Really appreciate the help. I understand it's tricky to debug this way. The complete error:

Uncaught Exception:
TypeError: Cannot read property 'id' of null
    at Timeout.makeScreenshot (<PATH>/node_modules/electron-screenshot-service/electron-service/node_modules/electron-screenshot-app/index.js:61:42)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)
    at listOnTimeoutNT (timers.js:242:26)
    at _combinedTickCallback (internal/process/next_tick.js:71:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
FWeinb commented 8 years ago

Just tried using Network Link Conditioner to throttle my connection to 3G but it still worked. There seams to be a race condition in the iframe detection code.

[Edit] Could you test inserting:

if (popupWindow === null) return;

in line 60 of index.js

peerbolte commented 8 years ago

I'm still getting an error when inserting the code: I really don't understand, shouldn't the return prohibit the code from executing?

Uncaught Exception:
TypeError: Cannot read property 'capturePage' of null
    at Timeout.setTimeout (<PATH>/node_modules/electron-screenshot-service/electron-service/node_modules/electron-screenshot-app/index.js:85:17)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)

[Edit] What version of node are you on? I'm at 5.2.0

[Edit2] It does seem to suppress some errors, however in my other code visiting example.com triggers the error mentioned above

[Edit3] I have suppressed all remaining errors by adding your suggestion at line 60 and 81

 if (popupWindow === null) return;

Everything seems to be working fine for now. Thank you so much for thinking along!

peerbolte commented 8 years ago

Are the if (popupWindow === null) return;solutions at line 60 and 81 valid enough to implement? As I think this solves the issue.

FWeinb commented 8 years ago

Added in electron-screenshot-app@3.1.1