Details
- Reviewers
kuba-orlik - Group Reviewers
Unknown Object (Project) - Maniphest Tasks
- T2741: Add a log of requests to the GUI
- Commits
- R134:1ea7b42e2979: added gui
Diff Detail
- Repository
- R134 rentgen-android
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
The server crashed for me while clicking n the gui:
android | INFO | Revoking microphone permissions for Google App.
http_server | node:internal/fs/utils:351
http_server | throw err;
http_server | ^
http_server |
http_server | Error: EBADF: bad file descriptor, write
http_server | at Object.writeSync (node:fs:952:3)
http_server | at Socket.<anonymous> (/code/index.js:23:7)
http_server | at Socket.emit (node:events:514:28)
http_server | at addChunk (node:internal/streams/readable:343:12)
http_server | at readableAddChunk (node:internal/streams/readable:316:9)
http_server | at Readable.push (node:internal/streams/readable:253:10)
http_server | at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
http_server | errno: -9,
http_server | syscall: 'write',
http_server | code: 'EBADF'
http_server | }
http_server |
http_server | Node.js v20.5.0
android | events.js:291
android | throw er; // Unhandled 'error' event
android | ^
android |
android | Error: write ECONNRESET
android | at afterWriteDispatched (internal/stream_base_commons.js:156:25)
android | at writeGeneric (internal/stream_base_commons.js:147:3)
android | at Socket._writeGeneric (net.js:788:11)
android | at Socket._write (net.js:800:8)
android | at doWrite (_stream_writable.js:403:12)
android | at writeOrBuffer (_stream_writable.js:387:5)
android | at Socket.Writable.write (_stream_writable.js:318:11)
android | at Socket.<anonymous> (/code/index.js:14:11)
android | at Socket.emit (events.js:314:20)
android | at addChunk (_stream_readable.js:297:12)
android | Emitted 'error' event on Socket instance at:
android | at errorOrDestroy (internal/streams/destroy.js:108:12)
android | at onwriteError (_stream_writable.js:418:5)
android | at onwrite (_stream_writable.js:445:5)
android | at internal/streams/destroy.js:50:7
android | at Socket._destroy (net.js:681:5)
android | at Socket.destroy (internal/streams/destroy.js:38:8)
android | at afterWriteDispatched (internal/stream_base_commons.js:156:17)
android | at writeGeneric (internal/stream_base_commons.js:147:3)
android | at Socket._writeGeneric (net.js:788:11)
android | at Socket._write (net.js:800:8) {
android | errno: 'ECONNRESET',
android | code: 'ECONNRESET',
android | syscall: 'write'
android | }
http_server exited with code 1
android exited with code 1And now it doesn't work when I boot it back:
npx zx start.mjs up $ docker compose build #0 building with "default" instance using docker driver #1 [http_server internal] load .dockerignore #1 transferring context: 2B done #1 DONE 0.0s #2 [http_server internal] load build definition from Dockerfile #2 transferring dockerfile: 126B done #2 DONE 0.0s #3 [android internal] load .dockerignore #3 transferring context: 2B done #3 DONE 0.0s #4 [android internal] load build definition from Dockerfile #4 transferring dockerfile: 758B done #4 DONE 0.1s #5 [proxy internal] load build definition from Dockerfile #5 transferring dockerfile: 102B done #5 DONE 0.1s #6 [proxy internal] load .dockerignore #6 transferring context: 2B done #6 DONE 0.1s #7 [proxy internal] load metadata for docker.io/mitmproxy/mitmproxy:9.0.1 #7 DONE 0.0s #8 [proxy 1/1] FROM docker.io/mitmproxy/mitmproxy:9.0.1 #8 CACHED #9 [proxy] exporting to image #9 exporting layers done #9 writing image sha256:4b56e25fcd3e8e238f2e41bd2480a9a6996893705347ca26d3f97ce7f142f770 done #9 naming to docker.io/library/rentgendroid-proxy done #9 DONE 0.0s #10 [android internal] load metadata for docker.io/runmymind/docker-android-sdk:ubuntu-standalone-20230511 #10 DONE 0.6s #11 [http_server internal] load metadata for docker.io/library/alpine:3.18.2 #11 DONE 0.5s #12 [http_server 1/3] FROM docker.io/library/alpine:3.18.2@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1 #12 DONE 0.0s #13 [http_server 2/3] RUN apk add npm #13 CACHED #14 [http_server 3/3] RUN mkdir /images #14 CACHED #15 [http_server] exporting to image #15 exporting layers done #15 writing image sha256:5ed6c48172dd729aceb8fb65874c0945b06ed2257b6cd61b269a96736cb11bca done #15 naming to docker.io/library/rentgendroid-http_server done #15 DONE 0.0s #16 [android 1/4] FROM docker.io/runmymind/docker-android-sdk:ubuntu-standalone-20230511@sha256:b63b081ff9ab747edb1074623c25433c724fdcdeb92dc2652f1afcf049d07370 #16 DONE 0.0s #17 [android 2/4] RUN sdkmanager "system-images;android-33;google_apis;x86_64" #17 CACHED #18 [android 3/4] RUN echo no | avdmanager create avd -n virtual_dev -b google_apis/x86_64 -k "system-images;android-33;google_apis;x86_64" #18 CACHED #19 [android 4/4] RUN apt-get update && apt-get install -y iproute2 iputils-ping iptables redsocks npm #19 CACHED #20 [android] exporting to image #20 exporting layers done #20 writing image sha256:21534535cce71b009f052207510d0fed31b06c25f563e207cf0ef348d883502d done #20 naming to docker.io/library/rentgendroid-android done #20 DONE 0.0s $ docker compose up Container http_server Created Container proxy Running Container android Created Attaching to android, http_server, proxy http_server | http_server | up to date, audited 60 packages in 971ms http_server | http_server | 8 packages are looking for funding http_server | run `npm fund` for details http_server | http_server | found 0 vulnerabilities android | android | up to date, audited 2 packages in 820ms android | android | found 0 vulnerabilities android | * daemon not running; starting now at tcp:5037 android | * daemon started successfully android | INFO | Android emulator version 32.1.12.0 (build_id 9751036) (CL:N/A) android | INFO | Found systemPath /opt/android-sdk-linux/system-images/android-33/google_apis/x86_64/ android | WARNING | System image is writable android | ERROR | Running multiple emulators with the same AVD android | ERROR | is an experimental feature. android | ERROR | Please use -read-only flag to enable this feature. android | INFO | Crashreporting disabled, not reporting crashes. android | INFO | Duplicate loglines will be removed, if you wish to see each indiviudal line launch with the -log-nofilter flag. proxy exited with code 0 proxy exited with code 137
| http_server/code/index.html | ||
|---|---|---|
| 10 | YOu can be more generous with the ids :) | |
| 28–30 | That seems like an overkill. I think you could just change the src to /screen?n=0 and replace 0 with an ever incrementing number and that should do it? The browser will handle the rest and we'll save ourselves from converting the image to base64 all the time | |
| http_server/code/index.html | ||
|---|---|---|
| 28–30 | the reason why i took this approach was so the backend didnt have to save the image and expose it through another endpoint, could that be done with the method you just described? | |
- added wait for the exposure of the endpoints till android fullt booted
- adding preact to the http_server / endpoint
- Reformat the README
- Fix the build script so it uses preact
- Move the jsx file to src
- Convert jsx to tsx
- Fix some ts issues
| android/conf/wait_for_sd.sh | ||
|---|---|---|
| 2 | When the android boots, the sd card inside the android emulator, where the screenshots are saved, takes some seconds to be ready, so we wait for the sd card to be ready before lauching the node script through which screenshots are taken, if we dont it will crash. | |
The endpoint seems to never become active for me. I've been waiting for 10 minutes, last output is
android | Now reboot your device for settings to take effect android | remount succeeded android | restarting adbd as root android | remount succeeded android | /c8750f0d.0: 1 file pushed, 0 skipped. 4.1 MB/s (1172 bytes in 0.000s) android | INFO | Boot completed in 45153 ms android | INFO | Increasing screen off timeout, logcat buffer size to 2M. android | INFO | Revoking microphone permissions for Google App.
| README.md | ||
|---|---|---|
| 15–16 | Let's maybe have a Makefile so we can just run make start to start the app? | |
| http_server/code/index.mjs | ||
| 20–24 | I worry about the shared state here. doneWrite is going to be shared across all pararell executions of the screenshot function. If there will be two subsequent requests to /screenshot, one will interfere with the other. We could add a safeguard like so: let screenshot_promise = null; async function guarded_screenshot(){ if(screenshot_promise){ return screenshot_promise; } screenshot_promise = screenshot(); return screenshot_promise; } | |
| 55–57 | ||
I couldn't get this to run - the wait for screenshots time to go below .5s timed out:
android | Using overlayfs for /product
android | Using overlayfs for /system_ext
android | Now reboot your device for settings to take effect
android | remount succeeded
android | restarting adbd as root
android | remount succeeded
android | /c8750f0d.0: 1 file pushed, 0 skipped. 10.4 MB/s (1172 bytes in 0.000s)
android | INFO | Boot completed in 45218 ms
android | INFO | Increasing screen off timeout, logcat buffer size to 2M.
android | INFO | Revoking microphone permissions for Google App.
android | listening on 3000
http_server | Waiting for full boot...
http_server | file:///code/index.mjs:48
http_server | throw new Error("wait for screenshot time to be less than 0.5s timed out");
http_server | ^
http_server |
http_server | Error: wait for screenshot time to be less than 0.5s timed out
http_server | at waitFullBoot (file:///code/index.mjs:48:8)
http_server | at async file:///code/index.mjs:65:1
http_server |
http_server | Node.js v20.5.0
http_server exited with code 1I ran this with docker-compose down && docker-compose build && npm install && npx zx start.mjs up
| README.md | ||
|---|---|---|
| 15–16 | We can then instead of a makefile create a package.json file that would let us use the npm start command. It's generally a good practice to have *some* shorthand way of running the scripts. It makes CI easier, as you just change what the start command does, you don't have to reconfigure the CI to run a different command, etc. | |
| android/code/index.js | ||
| 27–32 | I've just noticed that we're using spawnSync here. -Sync functions will block the entire execution of all javascript, which might make handling HTTP requests mor choppy. Let's create a small wrapper for spawn, something like async function spawnPromise(program, args) { return new Promise((resolve, reject) => { const process = child_process.spawn(program, args); process.on("close", (code) => { code == 0 ? reject() : resolve(); }); }); } and use that instead of spawnSync all across this file | |
| android/conf/install_cert.sh | ||
| 14 | What's the issue with the bypass? | |
| http_server/code/index.html | ||
| 12 | Let's follow the BEM class/id naming scheme: https://cssguidelin.es/#bem-like-naming (same applies for elements below) | |
| 46 | ||
| 47–48 | What are those arbitrary offsets? | |
I just tryed running it myself and it gives no problems, however i cannot manage to clone the latest version, but i took the one i have locally, removed everything that wasnt committed (all the .gitignore has), and had no problems, it didnt timeout. Please try again but run npm i && npx zx start.mjs down && npx zx start.mjs up, also tell me how can i clone the latest version. If the issue persists we should schedule a call to see whats wrong.
| android/code/index.js | ||
|---|---|---|
| 27–32 | i had the idea of using spawnsync bc i dont want 2 screenshot/tap processes to execute at the same time. Should i use this new function and await it? | |
| android/conf/install_cert.sh | ||
| 14 | Im not entirely sure, but 1 every 10 or 20 times it fails, i must dive deeper into it at some point | |
| http_server/code/index.html | ||
| 12 | understood, im on it | |
| 47–48 | when the screen gets displayed in the browser, it doesnt display in the (0,0), it displays in (8, 6), so those need to be substracted | |
| android/code/index.js | ||
|---|---|---|
| 27–32 | Yes, that would be preferable. In order then to prevent 2 screenshot processes from overlapping, we should implement a boolean flag that's raised while one process is running, and lowered when the process finishes, and use that to prevent double runs | |
| http_server/code/index.html | ||
| 47–48 | ah, you can use event.target.offsetX and offsetY in order to get those | |
Ok so I tested using this bash script:
#!/bin/bash cd /tmp git clone ssh://git@hub.sealcode.org:62022/source/rentgendroid.git cd rentgendroid arc patch D1354 npm i && npx zx start.mjs down && npx zx start.mjs up
And it times out waiting for full boot. Here's the output:
Just to be sure I checked out the current master branch and it boots successfully
| android/code/index.js | ||
|---|---|---|
| 27–32 | Blocking the main thread is a big antipattern in general. In this particular case it might not cause any apparent issues, but I want to avoid a situation where we have some performance problems in the future when adding new features and find this to be the culprit | |
| android/code/index.js | ||
|---|---|---|
| 27–32 | This is what I'm talking about - there's been a fluctuation in performance and too many requests got queued up.  They all wait for the response. Instead we could just reply with an error or a "please wait a bit" message | |
| http_server/code/index.html | ||
| 67 | Let's make it configurable via some env variable | |
| 67 | Also, using an interval is quite ruthless - meaning, if for some reason the request is going to take more than the interval, two requests are going to be sent. It'd be better to have something in line of async function start_screenshot_loop(){
while(true){
await refresh_screenshot();
await sleep(500)
}
} | |
| http_server/code/index.mjs | ||
| 43 | Let's make it configurable. Preferably we should just derive it from the same env variable that will configure the interval in index.html | |
| README.md | ||
|---|---|---|
| 17 | Pls document how to run it for... slower... machines like mine - that is, how to increase the screenshotDelayMs | |
| http_server/code/index.mjs | ||
| 101 | let's use the async version of readFile here. You can used the promise-friendly version with https://nodejs.org/api/fs.html#fspromisesreadfilepath-options | |