997320 - App actor front error with removeServerTemporaryFile

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

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

The switch to Promise.jsm in app-actor-front as part of bug 995170 seems to have brought this error to the surface:

Full message: TypeError: client.request is not a function
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
Full stack: removeServerTemporaryFile@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/app-actor-front.js:142:3
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:863:11
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:742:7

Looking at the code, I don't see how this ever worked.  It seems to be present in the first version of the file.

I guess this means there are many extra files on our phones...?! :)

Comment 1

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

Attachment: Fix error in App Manager install cleanup. r=ochameau

Comment 2

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

Comment on attachment 8407799 [details] [diff] [review]
Fix error in App Manager install cleanup. r=ochameau

So, thankfully are phones should not be full of files, because after uploading the file, the install actor moves it to a different directory for actual installation.

This patch actually calls remove as it was intended, and removes the "fail every time" callback that was present in |removeServerTemporaryFile|.  The "remove" call is likely to only have an effect if there was some error in the process.  Even then, we cleanup uploads on actor disconnect, so something really strange would have to happen.

Comment 3

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

Try: https://tbpl.mozilla.org/?tree=Try&rev=a6cac027a60a

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8407799 [details] [diff] [review]
Fix error in App Manager install cleanup. r=ochameau

Review of attachment 8407799 [details] [diff] [review]:
-----------------------------------------------------------------

Hopefully, the server code is smarter than the client!

::: toolkit/devtools/apps/app-actor-front.js
@@ +182,5 @@
>            // In case of success or error, ensure deleting the temporary package file
>            // also created on the device, but only once install request is done
> +          deferred.promise
> +              .then(() => removeServerTemporaryFile(client, fileActor),
> +                    () => removeServerTemporaryFile(client, fileActor));

nit: indentation of `.then`.

Comment 5

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

Attachment: Fix error in App Manager install cleanup. r=ochameau

Comment 6

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

Comment on attachment 8408332 [details] [diff] [review]
Fix error in App Manager install cleanup. r=ochameau

Fix indentation, carrying over r+ from attachment 8407799 [details] [diff] [review].

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ad0b2772d49

Comment 8

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/mozilla-central/rev/4ad0b2772d49

Comment 9

[Paul Rouget [:paul]]

I don't know if this is related or not, but today I'm getting this error:

*************************
A coding exception was thrown in a Promise resolution callback.

Full message: TypeError: client.request is not a function
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
Full stack: removeServerTemporaryFile@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/app-actor-front.js:142:3
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:748:11
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:627:7

*************************
onPacket threw an exception: Error: Server did not specify an actor, dropping packet: {"error":"unknownError","message":"error occurred while processing 'attach: ReferenceError: devtools is not defined\nStack: DebuggerProgressListener@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/actors/webbrowser.js:1264:1\nBTA_attach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/actors/webbrowser.js:676:5\nBTA_onAttach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/actors/webbrowser.js:744:5\nDSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1203:9\nChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/transport.js:347:5\nLine: 1264, column: 0"}
Stack: DebuggerClient.prototype.onPacket/<@resource://gre/modules/devtools/dbg-client.jsm:685:1
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
then@resource://gre/modules/commonjs/sdk/core/promise.js:153:9
DebuggerClient.prototype.onPacket@resource://gre/modules/devtools/dbg-client.jsm:735:1
DT__processIncoming/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/server/transport.js:201:9
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:86:7
Line: 685, column: 0

Comment 10

[Paul Rouget [:paul]]

Ok, just read the whole bug. Ignore my comment.