From fdab2a6c561aa4649f6f8dfdf675eaf90b8da551 Mon Sep 17 00:00:00 2001 From: Alexander Ackermann Date: Wed, 4 Nov 2026 06:36:08 +0100 Subject: [PATCH 1/2] fix: show not found in apps for an unknown space or a route without a file --- .../components/AppTemplates/AppWrapper.vue | 7 +++ .../AppTemplates/AppWrapper.spec.ts | 54 ++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue index 92ff71b56d..9f22b1ed0e 100644 --- a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue +++ b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue @@ -420,9 +420,16 @@ const addMissingDriveAliasAndItem = async () => { const loadResourceTask = useTask(function* (signal) { try { if (!unref(driveAliasAndItem)) { + if (!unref(fileId)) { + throw new HttpError('Resource not found', undefined, 404) + } yield addMissingDriveAliasAndItem() } space.value = unref(unref(currentFileContext).space) + if (!unref(space)) { + // the space is unknown or not accessible, like the server answers for such resources + throw new HttpError('Resource not found', undefined, 404) + } const currentSpace = unref(space) if (isPublicSpaceResource(currentSpace) && currentSpace.publicLinkPermission === undefined) { // the link permission and file id are only known if the link got resolved beforehand, diff --git a/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts b/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts index 43f9d9c7e1..0435ade2e2 100644 --- a/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts +++ b/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts @@ -126,6 +126,7 @@ function setup({ params: { driveAliasAndItem: 'personal/alan/a.md' } }), isGuestContext = false, + fileContext = mock({ space: mock(), path: '/a.md' }), fileContentOptions = undefined as FileContentOptions, // Receives the errors Vue would otherwise report as unhandled. errorHandler = undefined as (err: unknown) => void @@ -144,7 +145,7 @@ function setup({ isConflicted.value = true }) const statusRef = ref(status) - const currentFileContext = ref(mock({ space: mock(), path: '/a.md' })) + const currentFileContext = ref(fileContext) // Deferred so the test controls when each half of the load completes. let resolveInfo: (r: Resource) => void let rejectInfo: (e: Error) => void @@ -286,6 +287,7 @@ function setup({ markConflicted, putFileContents, closeApp, + getFileInfo, getFileContents, unregisterAppBarExtension, wasWrittenByRoom, @@ -436,6 +438,8 @@ describe('AppWrapper — missing drive alias', () => { const s = setup({ yjsEnabled: false, currentRoute: route as unknown as RouteLocation, + // the drive resolver only finds the space once the route has the drive alias + fileContext: { space: undefined, path: '' } as FileContext, errorHandler }) // Like the router: the new route changes the file context, which restarts @@ -456,6 +460,54 @@ describe('AppWrapper — missing drive alias', () => { }) }) +describe('AppWrapper — space that cannot be resolved', () => { + // e.g. the space of another user, whose alias the drive resolver doesn't know + const unresolvedFileContext = { space: undefined, path: '' } as FileContext + + // no mock: it would hand out a file id for any query item + function routeWithoutFileId(driveAliasAndItem: string) { + return { + path: '/test-app', + query: {}, + params: { driveAliasAndItem } + } as unknown as RouteLocation + } + + function expectNotFoundMessage(s: ReturnType) { + expect(s.wrapper.find('#app-wrapper-not-found-message').exists()).toBe(true) + expect(s.wrapper.findComponent({ name: 'ErrorScreen' }).exists()).toBe(false) + } + + it('shows the not found message instead of loading the file without a space', async () => { + const s = setup({ + yjsEnabled: false, + fileContext: unresolvedFileContext, + currentRoute: routeWithoutFileId('project/doesnotexist/x.md') + }) + await flushPromises() + + expect(s.getFileInfo).not.toHaveBeenCalled() + expect(getResourceContextSpy).not.toHaveBeenCalled() + expectNotFoundMessage(s) + }) + + it('shows the not found message for the app route without a file', async () => { + const errorHandler = vi.fn() + const s = setup({ + yjsEnabled: false, + fileContext: unresolvedFileContext, + currentRoute: routeWithoutFileId(''), + errorHandler + }) + await flushPromises() + + expect(getResourceContextSpy).not.toHaveBeenCalled() + expect(s.getFileInfo).not.toHaveBeenCalled() + expect(errorHandler).not.toHaveBeenCalled() + expectNotFoundMessage(s) + }) +}) + describe('AppWrapper — closing the app while the file is loading', () => { it('stops loading without an error', async () => { const errorHandler = vi.fn() From 2068d9bfa5da5e03e866c3a06c34884c2e4b723e Mon Sep 17 00:00:00 2001 From: Alexander Ackermann Date: Wed, 4 Nov 2026 06:42:35 +0100 Subject: [PATCH 2/2] refactor: use a dedicated error for resources that can't be found in apps --- .../web-pkg/src/components/AppTemplates/AppWrapper.vue | 10 +++++++--- packages/web-pkg/src/errors/types.ts | 7 +++++++ .../unit/components/AppTemplates/AppWrapper.spec.ts | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue index 9f22b1ed0e..2d6f43b427 100644 --- a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue +++ b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue @@ -103,6 +103,7 @@ import { DavPermission } from '@opencloud-eu/web-client/webdav' import { HttpError } from '@opencloud-eu/web-client' import { dirname } from 'path' import { useFileActionsOpenWithApp } from '../../composables' +import { ResourceNotFoundError } from '../../errors' import { UnsavedChangesModal } from '../Modals' import { decodeTextContent, @@ -175,7 +176,10 @@ const loading = ref(!unref(noResourceLoading)) const loadingError: Ref = ref() // apps that load their files themselves (e.g. the preview) report errors via the folder loading const error = computed(() => unref(loadingError) || unref(folderLoadingError)) -const isNotFoundError = computed(() => (unref(error) as HttpError)?.statusCode === 404) +const isNotFoundError = computed(() => { + const e = unref(error) + return e instanceof ResourceNotFoundError || (e as HttpError)?.statusCode === 404 +}) const isReadOnly = ref(false) /** The loaded text is not valid UTF-8, see `getFileContentsAsText`. */ const isLossyContent = ref(false) @@ -421,14 +425,14 @@ const loadResourceTask = useTask(function* (signal) { try { if (!unref(driveAliasAndItem)) { if (!unref(fileId)) { - throw new HttpError('Resource not found', undefined, 404) + throw new ResourceNotFoundError('No file given') } yield addMissingDriveAliasAndItem() } space.value = unref(unref(currentFileContext).space) if (!unref(space)) { // the space is unknown or not accessible, like the server answers for such resources - throw new HttpError('Resource not found', undefined, 404) + throw new ResourceNotFoundError('Space not found') } const currentSpace = unref(space) if (isPublicSpaceResource(currentSpace) && currentSpace.publicLinkPermission === undefined) { diff --git a/packages/web-pkg/src/errors/types.ts b/packages/web-pkg/src/errors/types.ts index 9e9571a369..2cd309c0ad 100644 --- a/packages/web-pkg/src/errors/types.ts +++ b/packages/web-pkg/src/errors/types.ts @@ -18,3 +18,10 @@ export class RuntimeError extends Error { export class ApiError extends RuntimeError { name = 'ApiError' } + +/** + * error for resources that can't be found without the server telling so, e.g. an unknown space + */ +export class ResourceNotFoundError extends RuntimeError { + name = 'ResourceNotFoundError' +} diff --git a/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts b/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts index 0435ade2e2..01069f98eb 100644 --- a/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts +++ b/packages/web-pkg/tests/unit/components/AppTemplates/AppWrapper.spec.ts @@ -18,6 +18,7 @@ import type { YjsSession, YjsStatus } from '../../../../src/composables/yjs' import type { FileContentOptions, FileContext } from '../../../../src/composables/appDefaults' import type { FileAction } from '../../../../src/composables/actions' import { eventBus } from '../../../../src/services' +import { ResourceNotFoundError } from '../../../../src/errors' import { type CustomComponentExtension, useExtensionRegistry, @@ -474,6 +475,7 @@ describe('AppWrapper — space that cannot be resolved', () => { } function expectNotFoundMessage(s: ReturnType) { + expect(consoleErrorSpy).toHaveBeenCalledWith(expect.any(ResourceNotFoundError)) expect(s.wrapper.find('#app-wrapper-not-found-message').exists()).toBe(true) expect(s.wrapper.findComponent({ name: 'ErrorScreen' }).exists()).toBe(false) }