nextcloud / nextcloud/android

File moves silently fail without any debug logging

Open
#11,773 2 comments 0 reactions 0 assignees View on GitHub
bug feature: auto upload hotspot: device storage
Dominant language
Kotlin
Stars
5.6k
Forks
2k
Avg merge
2d 18h
Merged PRs (30d)
92

Description

### ⚠️ Before posting ⚠️

- [X] This is a **bug**, not a question or an enhancement.
- [X] I've [searched for similar issues](https://github.com/nextcloud/android/issues) and didn't find a duplicate.
- [X] I've written a clear and descriptive title for this issue, not just "Bug" or "Crash".
- [X] I agree to follow Nextcloud's [Code of Conduct](https://nextcloud.com/contribute/code-of-conduct/).

### Steps to reproduce

I do not have enough information to demonstrate accurate testing conditions because there is no logging for this. (The logging specifically is what this issue is for.)

I frequently encounter files that I have selected as "Move to app folder" not being deleted after they have been uploaded, which leads to duplicates on my phone.

### Expected behaviour

Both times UploadFileOperation calls the `move` command, they're wrapped in a try-catch block.

```java
try {
move(temporalFile, expectedFile);
} catch (IOException e) {
Log_OC.e(TAG, e.getMessage());
}
```
```java
try {
move(originalFile, newFile);
} catch (IOException e) {
Log_OC.e(TAG, "Error moving file", e);
}
```

Therefore, all errors inside `move` should be fine to propagate up into those calls, since errors should be logged.

### Actual behaviour

Inside `move`, there is another try-catch, blocking the error from being propagated and therefore preventing it from being logged.

```java
try {
inChannel.transferTo(0, inChannel.size(), outChannel);
sourceFile.delete();
} catch (Exception e) {
mFile.setStoragePath(""); // forget the local file
// by now, treat this as a success; the file was uploaded
// the best option could be show a warning message
}
```

I **do not** want duplicate files on my phone. If the delete fails after the copy successes, the failed delete **should be logged.** If the error is not logged, then there's no way for me to assist the developers to identify why the delete operation failed.

Either the try-catch should be removed, or if you still need to call `mFile.setStoragePath("")` in this case, then I'd say just `throw e` after that call, so that the exception is logged, and so that we can figure out why the delete operation is failing.

### Android version

N/A

### Device brand and model

N/A

### Stock or custom OS?

Stock

### Nextcloud android app version

N/A (source)

### Nextcloud server version

N/A

### Using a reverse proxy?

No

### Android logs

N/A

### Server error logs

```bash
N/A
```

### Additional information

I'm not an Android developer and don't have a development environment set up, so I can't make this code modification myself and submit a PR. This is the best I can do at the moment. If this change is implemented, then I'll be able to analyze the debug logs and we'd be able to figure out why my files are being copied to the app folder but not deleted. When that happens, I can create a new issue for that. This issue is solely for the code change.

I'd like to explain, too, why I don't want duplicate files on my phone. From the perspective of a user (which I am), I have no way of verifying that _all_ of my uploads succeeded when the majority (or sometimes all) of my videos/photos stay in my DCIM/Camera folder. On multiple occasions, I had to manually verify that each and every one of my photos got uploaded successfully before manually deleting them from DCIM. Because from my perspective, if Nextcloud says _"I will move this file from the source folder to the Nextcloud folder when it's uploaded",_ and the file **remains in the source folder**, then the only logical conclusion is that Nextcloud **did not** upload the file. However, since the delete operation isn't logged when it fails, I'm currently unable to give you any useful information on fixing it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.