From 3b0fbd9efce04b2cd27994238de91fcb70edb415 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 28 Nov 2025 17:59:02 +0100 Subject: [PATCH] fix: enhance error handling and message extraction in WebDAV API #5486 --- src/app/imex/sync/get-sync-error-str.ts | 8 +- src/app/pfapi/api/errors/errors.ts | 91 +++++++++-- .../api/sync/providers/webdav/webdav-api.ts | 45 +++++- .../providers/webdav/webdav-base-provider.ts | 150 ++++++++++++++++++ .../providers/webdav/webdav-http-adapter.ts | 4 +- 5 files changed, 274 insertions(+), 24 deletions(-) create mode 100644 src/app/pfapi/api/sync/providers/webdav/webdav-base-provider.ts diff --git a/src/app/imex/sync/get-sync-error-str.ts b/src/app/imex/sync/get-sync-error-str.ts index bc68af106..df5c26317 100644 --- a/src/app/imex/sync/get-sync-error-str.ts +++ b/src/app/imex/sync/get-sync-error-str.ts @@ -6,6 +6,11 @@ export const getSyncErrorStr = (err: unknown): string => { let errorAsString: string = err && (err as any)?.toString ? (err as any).toString() : '???'; + // Check if error has a message property (most Error objects do) + if (err && typeof (err as any)?.message === 'string') { + errorAsString = (err as any).message as string; + } + if (err && typeof (err as any)?.response?.data === 'string') { errorAsString = (err as any)?.response?.data as string; } @@ -18,5 +23,6 @@ export const getSyncErrorStr = (err: unknown): string => { errorAsString = (err as any)[HANDLED_ERROR_PROP_STR] as string; } - return truncate(errorAsString.toString(), 150); + // Increased from 150 to 400 to show more context, especially for HTTP errors + return truncate(errorAsString.toString(), 400); }; diff --git a/src/app/pfapi/api/errors/errors.ts b/src/app/pfapi/api/errors/errors.ts index 04c640626..aef134d04 100644 --- a/src/app/pfapi/api/errors/errors.ts +++ b/src/app/pfapi/api/errors/errors.ts @@ -73,20 +73,83 @@ export class HttpNotOkAPIError extends AdditionalLogErrorBase { this.response = response; this.body = body; const statusText = response.statusText || 'Unknown Status'; - const safeBody = - typeof body === 'string' - ? body - : body !== undefined - ? (() => { - try { - return JSON.stringify(body); - } catch (e) { - return String(body); - } - })() - : ''; - const bodyText = safeBody ? ` - ${safeBody.substring(0, 300)}` : ''; - this.message = `HttpNotOkAPIError: ${response.status} ${statusText}${bodyText}`; + + // Parse body to extract meaningful error information + let errorDetail = ''; + if (body) { + const safeBody = + typeof body === 'string' + ? body + : body !== undefined + ? (() => { + try { + return JSON.stringify(body); + } catch (e) { + return String(body); + } + })() + : ''; + + // Try to extract meaningful error from XML/HTML responses + errorDetail = this._extractErrorFromBody(safeBody); + } + + const bodyText = errorDetail ? ` - ${errorDetail}` : ''; + this.message = `HTTP ${response.status} ${statusText}${bodyText}`; + } + + private _extractErrorFromBody(body: string): string { + if (!body) return ''; + + // Limit body length for error messages + const maxBodyLength = 300; + + // Try to extract error from Nextcloud/WebDAV XML responses + // Look for or tags + const nextcloudMessageMatch = body.match(/]*>(.*?)<\/s:message>/i); + if (nextcloudMessageMatch && nextcloudMessageMatch[1]) { + return nextcloudMessageMatch[1].trim().substring(0, maxBodyLength); + } + + const webdavErrorMatch = body.match(/]*>(.*?)<\/d:error>/i); + if (webdavErrorMatch && webdavErrorMatch[1]) { + return webdavErrorMatch[1].trim().substring(0, maxBodyLength); + } + + // Look for HTML title tags (often contain error descriptions) + const titleMatch = body.match(/]*>(.*?)<\/title>/i); + if (titleMatch && titleMatch[1]) { + const title = titleMatch[1].trim(); + // Avoid generic titles + if (title && !title.match(/^(error|404|403|500)$/i)) { + return title.substring(0, maxBodyLength); + } + } + + // Try to extract JSON error + try { + const jsonMatch = body.match(/\{.*\}/s); + if (jsonMatch) { + const parsed = JSON.parse(jsonMatch[0]); + if (parsed.error) { + return String(parsed.error).substring(0, maxBodyLength); + } + if (parsed.message) { + return String(parsed.message).substring(0, maxBodyLength); + } + } + } catch (e) { + // Not JSON, continue + } + + // Strip HTML tags for plain text + const withoutTags = body + .replace(/<[^>]+>/g, ' ') + .replace(/\s+/g, ' ') + .trim(); + + // Return the first meaningful chunk of text + return withoutTags.substring(0, maxBodyLength); } } diff --git a/src/app/pfapi/api/sync/providers/webdav/webdav-api.ts b/src/app/pfapi/api/sync/providers/webdav/webdav-api.ts index 4c3122871..85d79c655 100644 --- a/src/app/pfapi/api/sync/providers/webdav/webdav-api.ts +++ b/src/app/pfapi/api/sync/providers/webdav/webdav-api.ts @@ -54,17 +54,35 @@ export class WebdavApi { return meta; } } - - // If PROPFIND fails or returns no data, try HEAD request as fallback - if (useGetFallback) { - return await this._getFileMetaViaHead(fullPath); - } - - throw new RemoteFileNotFoundAPIError(path); } catch (e) { + // If PROPFIND fails and fallback is enabled, try HEAD + if (useGetFallback) { + PFLog.verbose( + `${WebdavApi.L}.getFileMeta() PROPFIND failed, trying HEAD fallback`, + e, + ); + try { + return await this._getFileMetaViaHead(fullPath); + } catch (headErr) { + PFLog.warn( + `${WebdavApi.L}.getFileMeta() HEAD fallback failed for ${path}`, + headErr, + ); + // If HEAD also fails, throw the original error (or maybe the HEAD error?) + // Usually the original PROPFIND error is more informative about connectivity + } + } PFLog.error(`${WebdavApi.L}.getFileMeta() error`, { path, error: e }); throw e; } + + // If we get here, PROPFIND worked but returned no data (or not MULTI_STATUS) + // Try HEAD request as fallback if enabled + if (useGetFallback) { + return await this._getFileMetaViaHead(fullPath); + } + + throw new RemoteFileNotFoundAPIError(path); } async download({ path }: { path: string }): Promise<{ @@ -507,6 +525,8 @@ export class WebdavApi { ); } + const etag = headers['etag'] || headers['ETag'] || ''; + return { filename, basename: filename, @@ -514,7 +534,16 @@ export class WebdavApi { size, type: contentType || 'application/octet-stream', etag: lastModified, // Use lastmod as etag for consistency - data: {}, + data: { + /* eslint-disable @typescript-eslint/naming-convention */ + 'content-type': contentType, + 'content-length': contentLength, + 'last-modified': lastModified, + /* eslint-enable @typescript-eslint/naming-convention */ + etag: etag, + href: fullPath, + }, + path: fullPath, // NEW: Populate the required path property }; } } diff --git a/src/app/pfapi/api/sync/providers/webdav/webdav-base-provider.ts b/src/app/pfapi/api/sync/providers/webdav/webdav-base-provider.ts new file mode 100644 index 000000000..1b9a0b4f5 --- /dev/null +++ b/src/app/pfapi/api/sync/providers/webdav/webdav-base-provider.ts @@ -0,0 +1,150 @@ +import { SyncProviderServiceInterface } from '../../sync-provider.interface'; +import { PrivateCfgByProviderId } from '../../../pfapi.model'; +import { SyncProviderId } from '../../../pfapi.const'; +import { WebdavApi } from './webdav-api'; +import { SyncProviderPrivateCfgStore } from '../../sync-provider-private-cfg-store'; +import { + InvalidDataSPError, + MissingCredentialsSPError, + NoRevAPIError, +} from '../../../errors/errors'; +import { WebdavPrivateCfg } from './webdav.model'; +import { SyncLog } from '../../../../../core/log'; + +/** + * Base class for WebDAV-based sync providers. + * Provides common functionality for uploading, downloading, and managing files via WebDAV. + * Extend this class to create specialized WebDAV providers (e.g., SuperSync). + */ +export abstract class WebdavBaseProvider< + T extends SyncProviderId.WebDAV | SyncProviderId.SuperSync, +> implements SyncProviderServiceInterface +{ + abstract readonly id: T; + readonly isUploadForcePossible = false; + readonly maxConcurrentRequests = 10; + + protected readonly _api: WebdavApi; + public privateCfg!: SyncProviderPrivateCfgStore; + + constructor(protected _extraPath?: string) { + this._api = new WebdavApi(() => this._cfgOrError()); + } + + /** + * Returns a label for logging purposes. + * Override in subclasses for more specific logging. + */ + protected get logLabel(): string { + return 'WebdavBaseProvider'; + } + + async isReady(): Promise { + const privateCfg = await this.privateCfg.load(); + return !!( + privateCfg && + privateCfg.userName && + privateCfg.baseUrl && + privateCfg.syncFolderPath && + privateCfg.password + ); + } + + async setPrivateCfg(privateCfg: WebdavPrivateCfg): Promise { + await this.privateCfg.setComplete(privateCfg as PrivateCfgByProviderId); + } + + async getFileRev( + targetPath: string, + localRev: string | null, + ): Promise<{ rev: string }> { + const { filePath } = await this._getConfigAndPath(targetPath); + const meta = await this._api.getFileMeta(filePath, localRev, true); + // Fallback to etag if lastmod is missing (some servers might behavior unexpectedly) + return { rev: meta.lastmod || meta.data?.etag || '' }; + } + + async uploadFile( + targetPath: string, + dataStr: string, + localRev: string, + isForceOverwrite: boolean = false, + ): Promise<{ rev: string }> { + SyncLog.debug(this.logLabel, 'uploadFile', { + targetPath, + localRev, + isForceOverwrite, + }); + const { filePath } = await this._getConfigAndPath(targetPath); + + const result = await this._api.upload({ + path: filePath, + data: dataStr, + isForceOverwrite: isForceOverwrite, + expectedRev: isForceOverwrite ? null : localRev, + }); + + if (!result.rev) { + throw new NoRevAPIError(); + } + + return { rev: result.rev }; + } + + async downloadFile( + targetPath: string, + ): Promise<{ rev: string; legacyRev?: string; dataStr: string }> { + SyncLog.debug(this.logLabel, 'downloadFile', { targetPath }); + const { filePath } = await this._getConfigAndPath(targetPath); + + const result = await this._api.download({ + path: filePath, + }); + + if (!result.dataStr && result.dataStr !== '') { + throw new InvalidDataSPError(targetPath); + } + if (typeof result.rev !== 'string') { + throw new NoRevAPIError(); + } + + return { rev: result.rev, legacyRev: result.legacyRev, dataStr: result.dataStr }; + } + + async removeFile(targetPath: string): Promise { + SyncLog.debug(this.logLabel, 'removeFile', { targetPath }); + const { filePath } = await this._getConfigAndPath(targetPath); + await this._api.remove(filePath); + } + + async listFiles(dirPath: string): Promise { + SyncLog.debug(this.logLabel, 'listFiles', { dirPath }); + const { filePath } = await this._getConfigAndPath(dirPath); + return this._api.listFiles(filePath); + } + + protected _getFilePath(targetPath: string, cfg: WebdavPrivateCfg): string { + const parts = cfg.syncFolderPath ? [cfg.syncFolderPath] : []; + if (this._extraPath) { + parts.push(this._extraPath); + } + parts.push(targetPath); + return parts.join('/').replace(/\/+/g, '/'); + } + + protected async _cfgOrError(): Promise { + const cfg = await this.privateCfg.load(); + if (!cfg) { + throw new MissingCredentialsSPError(); + } + return cfg; + } + + protected async _getConfigAndPath( + targetPath: string, + ): Promise<{ cfg: WebdavPrivateCfg; filePath: string }> { + const cfg = await this._cfgOrError(); + const filePath = this._getFilePath(targetPath, cfg); + return { cfg, filePath }; + } +} diff --git a/src/app/pfapi/api/sync/providers/webdav/webdav-http-adapter.ts b/src/app/pfapi/api/sync/providers/webdav/webdav-http-adapter.ts index b81d70a1f..d9515ffa7 100644 --- a/src/app/pfapi/api/sync/providers/webdav/webdav-http-adapter.ts +++ b/src/app/pfapi/api/sync/providers/webdav/webdav-http-adapter.ts @@ -176,8 +176,10 @@ export class WebDavHttpAdapter { if (status < 200 || status >= 300) { // Create a fake Response object for the error + // Ensure status is valid (200-599) for Response constructor + const safeStatus = status >= 200 && status <= 599 ? status : 500; const errorResponse = new Response('', { - status: status, + status: safeStatus, statusText: `HTTP ${status} for ${url}`, }); throw new HttpNotOkAPIError(errorResponse, body);