fix(svelte): Render correct error page (#63663)

Details about the probblem can be found in srch-527 (fixes srch-527).

This commit implements a workaround via the `handleError` hook. If this
function returns an error then SvelteKit is using that instead of its
own error object. So we unwrap the error object ourselves (since
SvelteKit fails to do that due to bazel build issues) and ensure this
way that the correct error is passed to our error template.

## Test plan

Coincidentally we disabled all error related tests in CI, so by enabling
these tests again we should see whether this approach works.

client/web-sveltekit/BUILD.bazel

@@ -214,7 +214,6 @@ playwright_test_bin.playwright_test(
     env = {
         "CHROMIUM_BIN": "$(rootpath //dev/tools:chromium)",
         "BAZEL": "1",
-        "BAZEL_SKIP_TESTS": "clone in progress;commit not found;not cloned;error loading commit information",
         "ASSETS_DIR": "./client/web-sveltekit/test_app_assets/test_build/_sk/",
     },
     flaky = True,

client/web-sveltekit/src/hooks.client.ts

@@ -1,6 +1,8 @@
 import { handleErrorWithSentry } from '@sentry/sveltekit'
 import * as Sentry from '@sentry/sveltekit'
 
+import { asError } from '$lib/common'
+
 Sentry.init({
     // Disabled if dsn is undefined
     dsn: window.context.sentryDSN ?? undefined,
@@ -25,5 +27,18 @@ Sentry.init({
     tracesSampleRate: 1.0,
 })
 
-// If you have a custom error handler, pass it to `handleErrorWithSentry`
-export const handleError = handleErrorWithSentry()
+export const handleError = handleErrorWithSentry(({ error }: { error: unknown }) => {
+    // When throwing _expected errors_ in data loaders via SvelteKit's `error`function,
+    // the error is wrapped in an object with `status` and `body` properties (an instance
+    // of `HTTPError`).
+    // Usually SvelteKit will unwrap the error itself and actually not call this function.
+    // But this doesn't work in production builds with bazel due to the fact that the `HTTPError`
+    // class is defined multiple times, which makes the `instanceof` check fail (it's not
+    // clear why the class is defined multiple times).
+    // By unwrapping and returning the error here we can still render the proper error message
+    // in the UI, otherwise it would show a generic "Internal Error" message.
+    if (error && typeof error === 'object' && 'body' in error) {
+        return error.body as Error
+    }
+    return asError(error)
+})

client/web-sveltekit/src/routes/+error.svelte

@@ -15,8 +15,8 @@
 {:else if isRevisionNotFoundErrorLike($page.error)}
     <RevisionNotFoundError />
 {:else}
-    <HeroPage title="Error" icon={ILucideCircleX}>
+    <HeroPage title="Unexpected Error" icon={ILucideCircleX}>
         <!-- TODO: format error message with markdown -->
-        {$page.error?.message ?? 'Unknown Error'}
+        {$page.error?.message ?? '(no error message)'}
     </HeroPage>
 {/if}

client/web-sveltekit/src/routes/[...repo=reporev]/-/commit/[...revspec]/page.spec.ts

@@ -18,12 +18,6 @@ test.beforeEach(async ({ sg }) => {
 })
 
 test('commit not found', async ({ page, sg }) => {
-    if (process.env.BAZEL_SKIP_TESTS?.includes('commit not found')) {
-        // Some tests are working with `pnpm run test` but not in Bazel.
-        // To get CI working we are skipping these tests for now.
-        // https://github.com/sourcegraph/sourcegraph/pull/62560#issuecomment-2128313393
-        return
-    }
     sg.mockOperations({
         ResolveRepoRevision: () => ({
             repositoryRedirect: {
@@ -44,12 +38,6 @@ test('commit not found', async ({ page, sg }) => {
 })
 
 test('error loading commit information', async ({ page, sg }) => {
-    if (process.env.BAZEL_SKIP_TESTS?.includes('error loading commit information')) {
-        // Some tests are working with `pnpm run test` but not in Bazel.
-        // To get CI working we are skipping these tests for now.
-        // https://github.com/sourcegraph/sourcegraph/pull/62560#issuecomment-2128313393
-        return
-    }
     sg.mockOperations({
         CommitPage_CommitQuery: () => {
             throw new Error('Test error')

client/web-sveltekit/src/routes/[...repo=reporev]/RepoNotFoundError.svelte

@@ -22,3 +22,10 @@
         <p>To access this repository, contact the Sourcegraph admin.</p>
     {/if}
 </HeroPage>
+
+<style lang="scss">
+    p {
+        text-align: center;
+        max-width: var(--viewport-md);
+    }
+</style>

client/web-sveltekit/src/routes/[...repo=reporev]/layout.spec.ts

@@ -43,12 +43,6 @@ test.describe('cloned repository', () => {
 })
 
 test('clone in progress', async ({ sg, page }) => {
-    if (process.env.BAZEL_SKIP_TESTS?.includes('clone in progress')) {
-        // Some tests are working with `pnpm run test` but not in Bazel.
-        // To get CI working we are skipping these tests for now.
-        // https://github.com/sourcegraph/sourcegraph/pull/62560#issuecomment-2128313393
-        return
-    }
     sg.mockOperations({
         ResolveRepoRevision: ({ repoName }) => ({
             repositoryRedirect: {
@@ -71,10 +65,6 @@ test('clone in progress', async ({ sg, page }) => {
 })
 
 test('not cloned', async ({ sg, page }) => {
-    if (process.env.BAZEL_SKIP_TESTS?.includes('not cloned')) {
-        // This test is flaky on CI
-        return
-    }
     sg.mockOperations({
         ResolveRepoRevision: ({ repoName }) => ({
             repositoryRedirect: {
@@ -96,6 +86,19 @@ test('not cloned', async ({ sg, page }) => {
     await expect(page.getByText('queued for cloning')).toBeVisible()
 })
 
+test('not found', async ({ sg, page }) => {
+    sg.mockOperations({
+        ResolveRepoRevision: () => ({
+            repositoryRedirect: null,
+        }),
+    })
+
+    await page.goto(`/${repoName}`)
+
+    // Shows not found error message
+    await expect(page.getByRole('heading', { name: 'Repository not found' })).toBeVisible()
+})
+
 test.describe('repo menu', () => {
     test.beforeEach(async ({ sg, page }) => {
         sg.mockOperations({