Panic in Screenshot

[yusufozturk]

Rod Version: v0.107.2

What you got

page.go:371 throws "invalid memory address or nil pointer deference"

I think "metrics" or "CSSContentSize" is nil. This happens in a customer environment, so I can't repro the issue when I want. I see the panic from the logs. I will try to do some debug but maybe you already have an idea which one might be nil?

What you expected to see

Instead of panic, it should return error.

What have you tried to solve the question

I can add nil check for metrics and CSSContentSize.

[rod-robot]

Please fix the format of your markdown:

9:244 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]

_{generated by check-issue}

[ysmood]

Can we create a fake HTML page to reproduce this?

[yusufozturk]

So as I understand, I should create a fake HTML page and PageGetLayoutMetricsResult should return nil for it?

I don't know how to do that yet. But I had 2 ideas:

1. Maybe customer has old Chrome version so only "ContentSize" filled in and not "CSSContentSize" ? It seems like "ContentSize" is deprecated.
2. Maybe just before PageGetLayoutMetricsResult request, browser process was killed? That can happen sometimes due to antivirus, 3rd party tools etc. In that case, entire "metrics" will be nil.

[ysmood]

About 2, it won't panic, because rod has protection for crashing, it will return proper err for it.

About 1, if we can't reproduce it, we won't merge the PR.

[yusufozturk]

Okay let me test it some more. If I can repro the issue, I can reopen the PR.

Thanks.

[yusufozturk]

@ysmood customer had an old Chrome version, so data was actually under ContentSize and CSSContentSize was nil.

We upgraded chrome version in that customer to avoid this issue. But there might be other people who has similar problem.
As the package owner, it's up to you if you want to add a nil check or not. I don't know how I can provide a test with an old version of Chrome but at least I can provide you some screenshots of the problematic Chrome.

PS: Please keep in mind, usually Enterprise has no internet access for internal networks. Chrome installations usually made by system engineers in the beginning of the server installation and no one updates it because nobody uses Chrome in the server and it's not in the patching plan. So it's normal to have old versions in some cases.

[ysmood]

Sure, we can make a PR for it, let's reopen it. Can you help to make the PR, thank you so much!