1391738 - Shift-F2 > screenshot fails with unknownError

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P3)

Description

[Emmanuel Gil Peyrot]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170818100226

Steps to reproduce:

- Go to any web page.
- Press Shift-F2.
- Type `screenshot webpage.png`


Actual results:

An “unknownError” message appeared at the bottom left of the page.


Expected results:

A screenshot should have been saved in ~/Downloads/webpage.png.

Comment 1

[YF (Yang)]

WFM in Fx57 20170919220202 on Win10.

Comment 2

[Nicolas Frandeboeuf]

Same thing here ! It appears it only fails with e10s.

Comment 3

[Henri Kemppainen]

console.error: 
  Message: Unix error 13 during operation stat on file name.png (Permission denied)
console.error: 
  unknownError

Indeed disabling e10s fixes the issue.

Comment 4

[Nicolas Frandeboeuf]

And it only fails when specifying "webpage.png". `screenshot` only will success.

Comment 6

[Soledad Penades [:sole] [:spenades]]

Matteo, do you have any insight into what's happening here?

It works fine on my Windows 10 machine too (except that I needed to press "Fn" as well as Shift + F2). My mac is unavailable right now, but I can test later.

Comment 7

[Nicolas Frandeboeuf]

I reproduce on Ubuntu 17.04 btw.

Comment 8

[Soledad Penades [:sole] [:spenades]]

It works fine on Mac OS. I am starting to suspect maybe we're trying to save the file to a place where files cannot be saved, e.g. maybe we're not finding the downloads directory properly and it's empty, and we're trying to store the image in / ... which is obviously not allowed!

Comment 9

[Henri Kemppainen]

The error happens well before we even try to save the screenshot.

Apparently running the command does some argument parsing, and for file type arguments, this involves

gcli/source/lib/gcli/util/fileparser.js.

It appears to call stat in filesystem.js, which in turn calls OS.File.stat() and fails with a permission error.  An exception is thrown and the whole command fails.

Is the file parser running in web content or some other restricted process?

I notice that if I pass /tmp/name.png, the thing will succeed.  If I pass /home/myuser/name.png, I get permission denied.

Comment 10

[Soledad Penades [:sole] [:spenades]]

I finally managed to set a Linux environment to debug this. I'm looking into this.

Comment 11

[Soledad Penades [:sole] [:spenades]]

Some investigations:

When you type "screenshot" as is, the command is run without analysing the arguments (because there aren't more). So a filename is generated and the screenshot is saved. All good.

When you type screenshot hello.png, it's slightly different.

As you type h...e...l... GCLI will be running stat on the filename (stat h... stat he...). These stat calls do not fail. It's the function in filesystem.js: http://searchfox.org/mozilla-central/rev/1033bfa26f6d42c1ef48621909f04e734a7ed8a3/devtools/shared/gcli/source/lib/gcli/util/filesystem.js#101

But when you have fully typed the command: screenshot hello.png and press enter, the argument is parsed again and stat is called again. This time this is happening on the content process (verified because the Browser Content Toolbox stopped on the breakpoint on the stat function only this time).

Then, the OS.File.stat function call fails. The onReject handler is called, but because it's not an instance of OS.File.Error, but a unixErrno: 13, the error is thrown. BAM! End of the story.

The stack for calling stat is:

stat - filesystem.js: 127
parse - fileparser.js: 35
parse - file.js: 88
setAssignment - cli.js: 1055
positionalDone - cli.js: 1972
promise - util.js: 279

Interestingly it is the same as when the code is called in non-content process (and it doesn't fail in that case)

Now, I am really not familiar with why this is happening, so I'm putting this here so I can ask for more help here O:-)

Comment 12

[Soledad Penades [:sole] [:spenades]]

Some more data: the tests are not catching this issue because they try to write to a file in the temporary directory, to which everyone has access.

In http://searchfox.org/mozilla-central/rev/1033bfa26f6d42c1ef48621909f04e734a7ed8a3/devtools/client/commandline/test/browser_cmd_screenshot.js#99 if I change "TmpD" to "Home" it will try to use a file in the user's home folder, AND fail!

Comment 13

[Soledad Penades [:sole] [:spenades]]

Attachment: Bug 1391738 - Screenshot with filename fails with unknown error in Linux.

Review commit: https://reviewboard.mozilla.org/r/187752/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/187752/

Comment 14

[Joe Walker [:jwalker]]

Comment on attachment 8916659 [details]
Bug 1391738 - Screenshot with filename fails with unknown error in Linux.

https://reviewboard.mozilla.org/r/187752/#review193110

Comment 15

[Soledad Penades [:sole] [:spenades]]

Green try https://treeherder.mozilla.org/#/jobs?repo=try&revision=e659d2f86dfcc83e795927638f931dfa19f19834

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/263a3798a798
Screenshot with filename fails with unknown error in Linux. r=jwalker

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/263a3798a798

Comment 18

[Soledad Penades [:sole] [:spenades]]

Comment on attachment 8916659 [details]
Bug 1391738 - Screenshot with filename fails with unknown error in Linux.

Approval Request Comment
[Feature/Bug causing the regression]: I am unable to determine this using mozregression, the last working build is 2016-10-06
[User impact if declined]: If users specify the filename when saving screenshots via DevTools GCLI feature, the feature doesn't work at all. Seems to have been broken for a long time :-/
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Using Linux and e10s, press Shift+F2 to open GCLI on a tab, then type screenshot hello.png - a screenshot of the current page will be saved to the Downloads folder.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: Not risky, we just changed the way the screenshot parameters are parsed, but added no new features or user visible changes
[String changes made/needed]: no

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8916659 [details]
Bug 1391738 - Screenshot with filename fails with unknown error in Linux.

Low risk, isolated change, beta57+

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0c7a2975f0a6

Comment 21

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Managed to reproduce the initial issue on 57.0a1 (2017-08-18). I can confirm that the webpage.png screenshot is successfully saved in ~/Downloads/ using 57.0b13 (20171030163911) and 58.0a1 (2017-10-31) on Ubuntu 16.06 x64. 
There is still an error triggered in Browser Console after the screenshot creation (see https://goo.gl/jDWMFu). I have to mention that this didn't show when the initial issue was reproducible. 
:sole, any thoughts about this?

Comment 22

[Soledad Penades [:sole] [:spenades]]

Iulia, I'm afraid I don't know what that is related to, it looks like something to do with the Download Manager

Comment 23

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Iulia Cristescu, QA [:JuliaC] from comment #21)
> Managed to reproduce the initial issue on 57.0a1 (2017-08-18). I can confirm
> that the webpage.png screenshot is successfully saved in ~/Downloads/ using
> 57.0b13 (20171030163911) and 58.0a1 (2017-10-31) on Ubuntu 16.06 x64. 
> There is still an error triggered in Browser Console after the screenshot
> creation (see https://goo.gl/jDWMFu). I have to mention that this didn't
> show when the initial issue was reproducible. 
> :sole, any thoughts about this?

I think this `setPageAnnotation` is a harmless error, at least as far as taking screenshots is concerned...  I believe it has been doing that for a while when screenshots are saved.  Seems similar to bug 1298362.