Firefox OS failed to boot with exception NS_ERROR_FILE_TARGET_DOES_NOT_EXIST

RESOLVED FIXED in mozilla25

Status

()

Core
DOM: Apps
P1
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: leo.bugzilla.gecko, Assigned: Changbin Park)

Tracking

Trunk
mozilla25
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
After I update my leo device with GOTA update, b2g process is not boot up with exception "NS_ERROR_FILE_TARGET_DOES_NOT_EXIST"
(Reporter)

Updated

4 years ago
Assignee: nobody → pchangbin
See Also: → bug 828835
(Assignee)

Comment 1

4 years ago
The GOTA update has a removed apps which is under /system/b2g/webapps/.
After GOTA update is performed the removed apps remains empty directories.

Steps to reproduce
 1. An app is installed from /system/b2g/webapps/<app> directory.
 2. Remove content of /system/b2g/webapps/<app> directory but remain the directory as empty.
 3. Make device performs update
   - To make it run installPreinstalledApp method in Webapps.jsm while booting.

NS_ERROR_FILE_TARGET_DOES_NOT_EXIST exception is occured while trying to copy /system/b2g/webapps/<app>/manifest.webapp and boot failed.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Created attachment 770576 [details] [diff] [review]
Prevent to make exception which make it stop running.

2 patches are applied.
One for detecting and skip empty directory in /system/b2g/webapps.
Another to catch a exception from copyTo method and keep it running.

If unhandled exception is occured in "installPreinstalledApp" method, it prevent boot process.
Attachment #770576 - Flags: review?(fabrice)
Severity: major → critical
Priority: -- → P1

Updated

4 years ago
Component: General → DOM: Apps
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Comment on attachment 770576 [details] [diff] [review]
Prevent to make exception which make it stop running.

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

::: dom/apps/src/Webapps.jsm
@@ +279,5 @@
>        baseDir = FileUtils.getDir("coreAppsDir", ["webapps", aId], false);
>        if (!baseDir.exists()) {
>          return;
> +      } else if (!baseDir.directoryEntries.hasMoreElements()) {
> +        dump("Error: Core app in " + baseDir.path + " is empty");

Nit: use debug() instead of dump().

@@ +320,5 @@
>          file.append(aFile);
> +        try {
> +          file.copyTo(destDir, aFile);
> +        } catch(e) {
> +          dump("Error: Failed to copy " + file.path + " to " + destDir.path);

Nit: Use debug() instead of dump().

In this case, we want to actually remove the app from the apps list, like we do when failing to extract something from the zip at the of the installPreinstalledApp() function.
Attachment #770576 - Flags: review?(fabrice) → review-
(Assignee)

Comment 4

4 years ago
(In reply to Fabrice Desré [:fabrice] from comment #3)
> Nit: use debug() instead of dump().
> Nit: Use debug() instead of dump().
I think debug() is better for consistency, but did you know the function is commented?
IMHO, the cases should not happened and it's about boot/update process.
So, if there's a case that messages are shown in log, it should be warned and checked.
That's why I used dump() instead of debug().
What do you think about it?
> 
> In this case, we want to actually remove the app from the apps list, like we
> do when failing to extract something from the zip at the of the
> installPreinstalledApp() function.
Like I said, the problem is occured while processing update.
So, aId is comes from /data/local/webapps/webapps.json. And it means application named "aId" is already installed.
Therefore, I think, the app should not be removed.
Think about that, the app was preinstalled app but after update it is not exist as preinstalled app anymore for some reason. And obviously, it's already installed.
Should I remove the app?

The empty directory is better be removed but the partition is mounted as RO.
So, I can't find any good solution for this so far.
Please, let me know if you have any good idea.
Flags: needinfo?(fabrice)
(In reply to Changbin Park from comment #4)
> (In reply to Fabrice Desré [:fabrice] from comment #3)
> > Nit: use debug() instead of dump().
> > Nit: Use debug() instead of dump().
> I think debug() is better for consistency, but did you know the function is
> commented?
> IMHO, the cases should not happened and it's about boot/update process.
> So, if there's a case that messages are shown in log, it should be warned
> and checked.
> That's why I used dump() instead of debug().
> What do you think about it?

We use inconditional dump() for severe errors, when the issue is not recoverable. That's not the case here so debug() is enough.

> > In this case, we want to actually remove the app from the apps list, like we
> > do when failing to extract something from the zip at the of the
> > installPreinstalledApp() function.
> Like I said, the problem is occured while processing update.
> So, aId is comes from /data/local/webapps/webapps.json. And it means
> application named "aId" is already installed.
> Therefore, I think, the app should not be removed.
> Think about that, the app was preinstalled app but after update it is not
> exist as preinstalled app anymore for some reason. And obviously, it's
> already installed.
> Should I remove the app?

As a user, I would not like the app to be removed, no.

> The empty directory is better be removed but the partition is mounted as RO.
> So, I can't find any good solution for this so far.
> Please, let me know if you have any good idea.

I'd like to understand better why we end up with an empty directory during update. Is this because the update mar file contains an empty directory (this would be a bug in the build system) or because we're not smart enough during the update process? The partition is mounted rw when applying updates.
Flags: needinfo?(fabrice)
(Assignee)

Comment 6

4 years ago
(In reply to Fabrice Desré [:fabrice] from comment #5)
> We use inconditional dump() for severe errors, when the issue is not
> recoverable. That's not the case here so debug() is enough.
OK.. I'll change it to "debug()". 

> I'd like to understand better why we end up with an empty directory during
> update. Is this because the update mar file contains an empty directory
> (this would be a bug in the build system) or because we're not smart enough
> during the update process? The partition is mounted rw when applying updates.
It's a kind of update process bug. As described in first comment, GOTA update makes the empty directory and it obviously a buf of GOTA update process. I'll be fixed anyway.
But, still, I think, it's better be got over such a abnormal case.
Because, it's too trivial abnormal case to prevent boot process.

It's better to have a smart solution to get rid of such a abnormal case, I couldn't find so far.
(Assignee)

Comment 7

4 years ago
Created attachment 775403 [details] [diff] [review]
Change dump() to debug() from previous patch

I change "dump()" to "debug()".
Attachment #770576 - Attachment is obsolete: true
Attachment #775403 - Flags: review?(fabrice)
Attachment #775403 - Flags: review?(fabrice) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Reporter)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/projects/birch/rev/2245d28611d6
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2245d28611d6
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.