New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 862040 link

Starred by 1 user
Status: Fixed
Owner:
lucmult@chromium.org
Closed: Jul 10
Cc:
noel@chromium.org
█ sashab@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Log error when getMetadata fails

Project Member Reported by lucmult@chromium.org, Jul 10 
Currently when getMetadata operation fails Files app just silently returns a default value.

This makes debugging issues with getMetadata hard to spot.

We should log the error before returning the default value to make issues with getMetadata more visible during development.

Important: We shouldn't use console.error() because in some conditions this error is sort of expected and it makes tests flaky, because console.error() increases a global error counter which is checked on tests. -  crbug.com/860647

Comment 1 by lucmult@chromium.org, Jul 10 

Some notes from  crbug.com/860647 :

"""
It turns out the error NotFound is for entry "/New Folder".

This error happens when "entry.getMetadata" is called after "/New Folder" has been already renamed to "/foo", thus the file system returns the "not found" error.

So, this error condition is sort of expected.

We have three options here.

1.  Don't add any logging and keep this failing silently and some issues with getMetadata can be tricky to find.

2.  Add logging as console.log(...) which doesn't make the test fail at all.

3.  Refactor prefetchMetadata and getMetadata to be able to be cancelled if the file has been renamed or deleted, but this seems a non-trivial change.
"""

We discussed offline and we want to proceed with #2 but with console.warn() instead of console.log().
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0be46892ccf3818f02e4bee4dc794be795aca389

commit 0be46892ccf3818f02e4bee4dc794be795aca389
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Tue Jul 10 05:06:14 2018

Add WARN logging for getMetadata error

Add the error and the file full path on the log, this allow easier
debugging for issues with getMetadata.

Bug:  862040 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I00226c0a472961d98dc923bfcce280c2f87bbd1d
Reviewed-on: https://chromium-review.googlesource.com/1128695
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573622}
[modify] https://crrev.com/0be46892ccf3818f02e4bee4dc794be795aca389/ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_provider.js

Comment 3 by lucmult@chromium.org, Jul 10

Status: Fixed (was: Started)
►

Sign in to add a comment