fix: say when an import stops before it is finished

.github/workflows/integration-test.yml

@@ -228,6 +228,9 @@ jobs:
           # a request without an access token has to be refused, otherwise the test could pass
           # while the app stopped sending the token
           test "$(curl -s -o /dev/null -w '%{http_code}' http://127.0.0.1:8099/v1.0/me/drive)" = "401"
+          # start from a healthy drive whatever an earlier run left behind
+          curl -fs -X POST http://127.0.0.1:8099/control/repair-drive
+          echo
 
       - name: Run Nextcloud
         working-directory: server/
@@ -422,6 +425,52 @@ jobs:
           fi
           exit $fail
 
+      - name: Import with a drive that cannot be reached
+        working-directory: server/
+        run: |
+          php occ user:setting admin ${{ env.APP_NAME }} onedrive_output_dir "/OneDrive stopped"
+          # drop what the previous imports sent, the one checked below has to be this import's
+          curl -s -X DELETE -u admin:admin -H 'OCS-APIRequest: true' \
+            'http://localhost:8080/ocs/v2.php/apps/notifications/api/v2/notifications' >/dev/null
+          curl -fs -X POST http://127.0.0.1:8099/control/break-drive
+          echo
+          # make sure the drive really is unreachable, otherwise the import would just work
+          test "$(curl -s -o /dev/null -w '%{http_code}' -H 'Authorization: bearer stub-access-token' http://127.0.0.1:8099/v1.0/me/drive)" = "403"
+          php occ ${{ env.APP_NAME }}:start-import admin
+          fail=0
+          importing=1
+          # a queued job is not always picked by the first cron run
+          for run in {1..4}; do
+            php cron.php
+            importing=$(php occ user:setting admin ${{ env.APP_NAME }} importing_onedrive 2>/dev/null | tail -1 || echo "1")
+            echo "round $run: importing=$importing"
+            if [ "$importing" = "0" ]; then break; fi
+          done
+          if [ "$importing" != "0" ]; then
+            echo "the import should have given up, importing_onedrive is '$importing'"
+            fail=1
+          fi
+          notification=$(curl -s -u admin:admin -H 'OCS-APIRequest: true' -H 'Accept: application/json' \
+            'http://localhost:8080/ocs/v2.php/apps/notifications/api/v2/notifications')
+          expected='The import of your OneDrive files stopped before it was finished, check the server logs for details. | 0 files were imported from OneDrive storage.'
+          said=$(php -r 'foreach (json_decode(stream_get_contents(STDIN), true)["ocs"]["data"] as $n) { if (($n["app"] ?? "") === "integration_onedrive") { echo $n["subject"], " | ", $n["message"] ?? "", "\n"; } }' <<< "$notification")
+          echo "$said"
+          if [ "$(printf '%s\n' "$said" | grep -c .)" != "1" ]; then
+            echo "expected exactly one notification of this import"
+            fail=1
+          fi
+          if [ "$said" != "$expected" ]; then
+            echo "expected: $expected"
+            fail=1
+          fi
+          # the reason belongs in that line, it is what the notification sends the admin to
+          if ! grep -q 'import of admin stopped: .*accessDenied' data/nextcloud.log; then
+            echo "the log does not say why the import stopped"
+            grep 'import of admin stopped' data/nextcloud.log || true
+            fail=1
+          fi
+          exit $fail
+
       - name: Show the logs on failure
         working-directory: server/
         if: failure()

CHANGELOG.md

@@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Stop counting the files an import downloaded itself among the files that were already there
 - Try a folder whose listing failed again in a later batch instead of leaving it out of the import
 - List a folder again from its first page when the page an import stopped in cannot be listed any more
+- Tell the user when an import stops before it has seen the whole drive, it used to end in silence, and write the reason to the log
+- Say nothing about an import the user cancelled while it was running, it used to report itself as finished
 - Test the import against a stubbed Graph API, including failed downloads, empty files and paging
 
 ## [3.5.2] - 2026-07-28

lib/Notification/Notifier.php

@@ -8,6 +8,7 @@
 namespace OCA\Onedrive\Notification;
 
 use OCA\Onedrive\AppInfo\Application;
+use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
@@ -65,6 +66,28 @@ public function getName(): string {
 		return $this->factory->get('integration_onedrive')->t('OneDrive');
 	}
 
+	/**
+	 * The sentences an import adds about the files it did not bring, if there were any.
+	 */
+	private function whatElseHappened(IL10N $l, int $nbSkipped, int $nbFailed): string {
+		$content = '';
+		if ($nbSkipped > 0) {
+			$content .= ' ' . $l->n(
+				'%n file was already there.',
+				'%n files were already there.',
+				$nbSkipped
+			);
+		}
+		if ($nbFailed > 0) {
+			$content .= ' ' . $l->n(
+				'%n file could not be downloaded, check the server logs for details.',
+				'%n files could not be downloaded, check the server logs for details.',
+				$nbFailed
+			);
+		}
+		return $content;
+	}
+
 	/**
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
@@ -89,21 +112,8 @@ public function prepare(INotification $notification, string $languageCode): INot
 				$nbSkipped = (int)($p['nbSkipped'] ?? 0);
 				$failedFiles = is_array($p['failedFiles'] ?? null) ? $p['failedFiles'] : [];
 				$targetPath = $p['targetPath'];
-				$content = $l->n('%n file was imported from OneDrive storage.', '%n files were imported from OneDrive storage.', $nbImported);
-				if ($nbSkipped > 0) {
-					$content .= ' ' . $l->n(
-						'%n file was already there.',
-						'%n files were already there.',
-						$nbSkipped
-					);
-				}
-				if ($nbFailed > 0) {
-					$content .= ' ' . $l->n(
-						'%n file could not be downloaded, check the server logs for details.',
-						'%n files could not be downloaded, check the server logs for details.',
-						$nbFailed
-					);
-				}
+				$content = $l->n('%n file was imported from OneDrive storage.', '%n files were imported from OneDrive storage.', $nbImported)
+					. $this->whatElseHappened($l, $nbSkipped, $nbFailed);
 
 				if ($failedFiles !== []) {
 					$names = implode(', ', $failedFiles);
@@ -118,6 +128,22 @@ public function prepare(INotification $notification, string $languageCode): INot
 					->setIcon($this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'app-dark.svg')))
 					->setLink($this->url->linkToRouteAbsolute('files.view.index', ['dir' => $targetPath]));
 				return $notification;
+			case 'import_onedrive_stopped':
+				/** @var array{nbImported?: string, nbFailed?: string, nbSkipped?: string, targetPath: string} $p */
+				$p = $notification->getSubjectParameters();
+				$nbImported = (int)($p['nbImported'] ?? 0);
+				$nbFailed = (int)($p['nbFailed'] ?? 0);
+				$nbSkipped = (int)($p['nbSkipped'] ?? 0);
+				$targetPath = $p['targetPath'];
+				$notification
+					->setParsedSubject($l->t('The import of your OneDrive files stopped before it was finished, check the server logs for details.'))
+					->setParsedMessage(
+						$l->n('%n file was imported from OneDrive storage.', '%n files were imported from OneDrive storage.', $nbImported)
+						. $this->whatElseHappened($l, $nbSkipped, $nbFailed)
+					)
+					->setIcon($this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'app-dark.svg')))
+					->setLink($this->url->linkToRouteAbsolute('files.view.index', ['dir' => $targetPath]));
+				return $notification;
 			default:
 				// Unknown subject => Unknown notification => throw
 				throw new UnknownNotificationException();

lib/Service/OnedriveAPIService.php

@@ -233,8 +233,10 @@ public function request(string $userId, string $endPoint, array $params = [], st
 				}
 			}
 		} catch (ServerException|ClientException $e) {
-			$this->logger->warning('OneDrive API error : ' . $e->getResponse()->getBody(), ['app' => Application::APP_ID]);
-			return ['error' => $e->getResponse()->getBody()];
+			// a string, so that whoever reports this error does not have to read a stream
+			$body = (string)$e->getResponse()->getBody();
+			$this->logger->warning('OneDrive API error : ' . $body, ['app' => Application::APP_ID]);
+			return ['error' => $body];
 		} catch (ConnectException $e) {
 			$this->logger->warning('OneDrive API connection error : ' . $e->getMessage(), ['app' => Application::APP_ID]);
 			return ['error' => $e->getMessage()];

lib/Service/OnedriveStorageAPIService.php

@@ -215,11 +215,15 @@ public function importOnedriveJob(string $userId): void {
 		try {
 			$result = $this->importFiles($userId, $targetPath, $batchSize, $alreadyImportedSize, $alreadyImportedNumber, $importTree);
 		} catch (Exception|Throwable $e) {
+			$this->logger->error('OneDrive import job failed: ' . $e->getMessage(), ['app' => Application::APP_ID, 'exception' => $e]);
 			$result = [
-				'error' => 'Unknow job failure. ' . $e->getMessage(),
+				'error' => $e->getMessage(),
 			];
 		}
 		if (isset($result['error']) || (isset($result['finished']) && $result['finished'])) {
+			// the settings page cancels an import by clearing this while a batch runs: the
+			// user knows it is over and does not need to hear about it
+			$cancelled = $this->config->getUserValue($userId, Application::APP_ID, 'importing_onedrive', '0') !== '1';
 			// read the counters accumulated over all batches before resetting them
 			$nbImported = (int)$this->config->getUserValue($userId, Application::APP_ID, 'nb_imported_files', '0');
 			$nbFailed = (int)$this->config->getUserValue($userId, Application::APP_ID, 'nb_failed_files', '0');
@@ -232,15 +236,27 @@ public function importOnedriveJob(string $userId): void {
 			$this->config->setUserValue($userId, Application::APP_ID, 'nb_skipped_files', '0');
 			$this->config->deleteUserValue($userId, Application::APP_ID, 'failed_files');
 			$this->config->setUserValue($userId, Application::APP_ID, 'last_onedrive_import_timestamp', '0');
-			if (isset($result['finished']) && $result['finished']) {
-				$this->config->deleteUserValue($userId, Application::APP_ID, 'import_tree');
+			$this->config->deleteUserValue($userId, Application::APP_ID, 'import_tree');
+			if ($cancelled) {
+				$this->logger->info('The OneDrive import of ' . $userId . ' was cancelled', ['app' => Application::APP_ID]);
+			} elseif (isset($result['finished']) && $result['finished']) {
 				$this->onedriveApiService->sendNCNotification($userId, 'import_onedrive_finished', [
 					'nbImported' => $nbImported,
 					'nbFailed' => $nbFailed,
 					'nbSkipped' => $nbSkipped,
 					'failedFiles' => $failedFiles,
 					'targetPath' => $targetPath,
 				]);
+			} else {
+				// the import ends here without having seen the whole drive, say so instead
+				// of leaving the user with an import that stopped for no visible reason
+				$this->logger->error('OneDrive import of ' . $userId . ' stopped: ' . $result['error'], ['app' => Application::APP_ID]);
+				$this->onedriveApiService->sendNCNotification($userId, 'import_onedrive_stopped', [
+					'nbImported' => $nbImported,
+					'nbFailed' => $nbFailed,
+					'nbSkipped' => $nbSkipped,
+					'targetPath' => $targetPath,
+				]);
 			}
 		} else {
 			// save progress

tests/integration/graph-stub.php

@@ -23,6 +23,9 @@
  *   already.txt  is never downloaded, the test puts it in the target folder beforehand
  *   sub/         a folder, holding nested.txt, which downloads
  *
+ * A request to /control/break-drive makes the drive itself unreachable, which is how an
+ * import that stops before it is finished is produced.
+ *
  * The first listing page carries an @odata.nextLink, so paging is covered as well.
  */
 
@@ -73,6 +76,14 @@ function baseUrl(): string {
 	return 'http://' . ($_SERVER['HTTP_HOST'] ?? '127.0.0.1:8099');
 }
 
+/**
+ * The file that says the drive has been made unreachable on purpose. It carries the port
+ * so that two stubs on one machine, or a later run, do not inherit it.
+ */
+function brokenDriveFlag(): string {
+	return sys_get_temp_dir() . '/graph-stub-broken-drive-' . ($_SERVER['SERVER_PORT'] ?? 'x');
+}
+
 function respond(array $body, int $status = 200): void {
 	http_response_code($status);
 	header('Content-Type: application/json');
@@ -113,6 +124,18 @@ function serveDownload(string $id): void {
 $path = parse_url($_SERVER['REQUEST_URI'] ?? '/', PHP_URL_PATH) ?: '/';
 $path = rawurldecode($path);
 
+// the test asks for the drive to be broken and repaired, no access token involved
+if ($path === '/control/break-drive') {
+	touch(brokenDriveFlag());
+	respond(['broken' => true]);
+	return;
+}
+if ($path === '/control/repair-drive') {
+	@unlink(brokenDriveFlag());
+	respond(['broken' => false]);
+	return;
+}
+
 // downloads carry their own authorisation in the URL, everything else needs the access token
 if (!str_starts_with($path, '/download/') && !preg_match('/^bearer .+/i', $_SERVER['HTTP_AUTHORIZATION'] ?? '')) {
 	respond(['error' => ['code' => 'unauthenticated', 'message' => 'no access token']], 401);
@@ -125,6 +148,11 @@ function serveDownload(string $id): void {
 }
 
 if ($path === '/v1.0/me/drive') {
+	if (file_exists(brokenDriveFlag())) {
+		// what a revoked consent looks like: the import cannot even read the drive
+		respond(['error' => ['code' => 'accessDenied', 'message' => 'the drive is not yours any more']], 403);
+		return;
+	}
 	respond(['id' => 'stub-drive', 'quota' => ['total' => 1073741824, 'used' => 42, 'remaining' => 1073741782]]);
 	return;
 }

tests/unit/Notification/NotifierTest.php

@@ -141,4 +141,23 @@ public function testThrowsForUnknownSubject(): void {
 		$this->expectException(UnknownNotificationException::class);
 		$this->notifier->prepare($notification, 'en');
 	}
+
+	public function testStoppedImportSaysWhatItGotDone(): void {
+		$this->prepare(
+			['nbImported' => 12, 'nbFailed' => 0, 'nbSkipped' => 0, 'targetPath' => '/OneDrive import'],
+			'The import of your OneDrive files stopped before it was finished, check the server logs for details.',
+			'12 files were imported from OneDrive storage.',
+			'import_onedrive_stopped'
+		);
+	}
+
+	public function testStoppedImportCountsWhatItSkippedAndCouldNotDownload(): void {
+		$this->prepare(
+			['nbImported' => 1, 'nbFailed' => 2, 'nbSkipped' => 3, 'targetPath' => '/OneDrive import'],
+			'The import of your OneDrive files stopped before it was finished, check the server logs for details.',
+			'1 file was imported from OneDrive storage. 3 files were already there.'
+			. ' 2 files could not be downloaded, check the server logs for details.',
+			'import_onedrive_stopped'
+		);
+	}
 }