1093108 - Log shaking dumps erases logs if a folder with the same name exists

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Johan Lorenzo [:jlorenzo]]

Pre-requisites
Have shake to log enabled

STR
1. Shake your device!
2. Retrieve your logs to a computer and note at what time they were save (YYYY-MM-DD-hh-mm-ss format)
3. Change the timezone to another and set the time to the one you had in 2.
4. Shake it up until your create logs with the same timestamp as in step 2.
5. Go to the logs folder

Expected result
You would have 2 folders to get the logs in step 1 and the ones in step 4.

Actual
The logs in step 2 are deleted. You can check for instance that dev-log-main.log doesn't have the same size in both cases (mine in step 4 were smaller than in step 1).

This is an edge case, that might be problematic edge case if you move through timezones.

Comment 1

[Johan Lorenzo [:jlorenzo]]

As a similar case: screenshots don't have this issue. When you try to overwrite one (because of the same timestamp), you get the notification "Screenshot could not be saved NoModificationAllowedError".

Comment 2

[:gerard-majax]

I do see another usecase where it could be much more problematic: crossing timezones.

Maybe we should rather leave some time between two shaking? Does it makes sense that we do constant dumping if the users does constant shaking ?

Comment 3

[:gerard-majax]

After offline discussion with johann, we came to the conclusion this is far from being a frequent edge case, so not working on this with high priority. If anyone comes with STRs that makes it more frequent, please raise your voice.

Comment 4

[:gerard-majax]

Attachment: Ensure LogShake will not overwrite pre-existing directories r=gwagner

The directory we are using to produce logs used by LogShake is computed
using local date/time. If for some reason the clock changes (traversing
timezones, daylight saving, bogus hardware, clock readjustment, ...) and
we are unlucky enough, then we may endup trying to write logs in a
directory that aready exists. We fix this by checking if there is
already a directory with this name, and if it turns out to be true, then
we append a digit to differentiate.

Comment 5

[:gerard-majax]

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

Comment 6

[:gerard-majax]

Attachment: Ensure LogShake will not overwrite pre-existing directories r=gwagner

The directory we are using to produce logs used by LogShake is computed
using local date/time. If for some reason the clock changes (traversing
timezones, daylight saving, bogus hardware, clock readjustment, ...) and
we are unlucky enough, then we may endup trying to write logs in a
directory that aready exists. We fix this by checking if there is
already a directory with this name, and if it turns out to be true, then
we append a digit to differentiate.

Comment 7

[:gerard-majax]

Damn, OS.File.exists() returns a promise .... Everything is screwed.

Comment 8

[:gerard-majax]

So I tried just making use of OS.File.makeDir() with "ignoreExisting: false". Yet, this is not working and the directory is overwritten.

Yoric, do you see anything wrong in b2g/components/LogShake.jsm, saveLogs() function ? Below is the matching logcat output, where I forced the directory to be "timestamp" instead of the real timestamp value.

12-16 19:09:08.541  6162  6162 I GeckoDump: LogShake: handling event capture-logs-start
12-16 19:09:08.541  6162  6162 I GeckoDump: LogShake: handling capture-logs-start
12-16 19:09:09.071  6162  6162 I Gecko   : LogShake.jsm: making a directory all the way from /storage/sdcard to /storage/sdcard/logs/timestamp
12-16 19:09:09.191  6490  6490 I Gecko   : ###################################### forms.js loaded
12-16 19:09:09.261  6490  6490 I Gecko   : ############################### browserElementPanning.js loaded
12-16 19:09:09.311  6490  6490 I Gecko   : ######################## BrowserElementChildPreload.js loaded
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of properties
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/main
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/system
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/radio
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/events
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/cmdline
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/kmsg
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/meminfo
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/uptime
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/version
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmallocinfo
12-16 19:09:09.621  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmstat
12-16 19:09:09.841  6162  6162 I Gecko   : LogShake.jsm: returning logfilenames: ["logs/timestamp/properties.log", "logs/timestamp/dev-log-main.log", "logs/timestamp/dev-log-system.log", "logs/timestamp/dev-log-radio.log", "logs/timestamp/dev-log-events.log", "logs/timestamp/proc-cmdline.log", "logs/timestamp/proc-kmsg.log", "logs/timestamp/proc-meminfo.log", "logs/timestamp/proc-uptime.log", "logs/timestamp/proc-version.log", "logs/timestamp/proc-vmallocinfo.log", "logs/timestamp/proc-vmstat.log"]
12-16 19:09:09.841  6162  6162 I GeckoDump: LogShake: handling event capture-logs-success
12-16 19:09:09.841  6162  6162 I GeckoDump: LogShake: handling capture-logs-success

...

12-16 19:09:11.901  6162  6162 I GeckoDump: LogShake: handling event capture-logs-start
12-16 19:09:11.901  6162  6162 I GeckoDump: LogShake: handling capture-logs-start
12-16 19:09:12.381  6162  6162 I Gecko   : LogShake.jsm: making a directory all the way from /storage/sdcard to /storage/sdcard/logs/timestamp
12-16 19:09:12.451  6162  6162 I Gecko   : LogShake.jsm: requesting save of properties
12-16 19:09:12.451  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/main
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/system
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/radio
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/events
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/cmdline
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/kmsg
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/meminfo
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/uptime
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/version
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmallocinfo
12-16 19:09:12.481  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmstat
12-16 19:09:12.781  6162  6162 I Gecko   : LogShake.jsm: returning logfilenames: ["logs/timestamp/properties.log", "logs/timestamp/dev-log-main.log", "logs/timestamp/dev-log-system.log", "logs/timestamp/dev-log-radio.log", "logs/timestamp/dev-log-events.log", "logs/timestamp/proc-cmdline.log", "logs/timestamp/proc-kmsg.log", "logs/timestamp/proc-meminfo.log", "logs/timestamp/proc-uptime.log", "logs/timestamp/proc-version.log", "logs/timestamp/proc-vmallocinfo.log", "logs/timestamp/proc-vmstat.log"]
12-16 19:09:12.781  6162  6162 I GeckoDump: LogShake: handling event capture-logs-success
12-16 19:09:12.781  6162  6162 I GeckoDump: LogShake: handling capture-logs-success

Comment 9

[:gerard-majax]

Attachment: Do not overwrite pre-existing logs r=gwagner

Simply rely on OS.File.makeDir() and gracefully fail instead of
overwriting pre-existing user data.

Comment 10

[David Teller [:Yoric] - still alive but not very active]

As discussed off-bugzilla, the issue is actually in the documentation of option `from` of `OS.File.makeDir`. Clearing needinfo.

Comment 11

[:gerard-majax]

Attachment: Do not overwrite pre-existing logs r=gwagner

Simply rely on OS.File.makeDir() and gracefully fail instead of
overwriting pre-existing user data.

Comment 12

[Carsten Book [:Tomcat]]

this needs a try run i guess ? Could you provide one? Thanks!

Comment 13

[:gerard-majax]

(In reply to Carsten Book [:Tomcat] from comment #12)
> this needs a try run i guess ? Could you provide one? Thanks!

Right, the one in comment 5 is not for the latest patch.

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

Comment 14

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #13)
> (In reply to Carsten Book [:Tomcat] from comment #12)
> > this needs a try run i guess ? Could you provide one? Thanks!
> 
> Right, the one in comment 5 is not for the latest patch.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=7911e19b51a7

And it's green :)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/9592f4f944bc

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9592f4f944bc