Merge pull request #1176 from vrtmrz/fix/issue-1166-r2-initialisation

Fix first-time R2 setup through the Custom HTTP Handler

package.json

@@ -69,6 +69,7 @@
         "test:e2e:obsidian:cli-to-obsidian-sync": "tsx test/e2e-obsidian/scripts/cli-to-obsidian-sync.ts",
         "test:e2e:obsidian:minio-upload": "tsx test/e2e-obsidian/scripts/minio-upload.ts",
         "test:e2e:obsidian:object-storage-setup-uri-workflow": "tsx test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts",
+        "test:e2e:obsidian:object-storage-custom-http-handler-setup-uri-workflow": "tsx test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts --custom-http-handler",
         "test:e2e:obsidian:p2p-setup-uri-workflow": "tsx test/e2e-obsidian/scripts/p2p-setup-uri-workflow.ts",
         "pretest:e2e:obsidian:p2p-connection-check": "npm run build && npm run build --workspace webpeer",
         "test:e2e:obsidian:p2p-connection-check": "tsx test/e2e-obsidian/scripts/p2p-connection-check.ts",

src/modules/essentialObsidian/APILib/ObsHttpHandler.ts

@@ -1,7 +1,7 @@
-// This file is based on a file that was published by the @remotely-save, under the Apache 2 License.
+// This file was originally based on code published by @remotely-save under the Apache License 2.0.
-// I would love to express my deepest gratitude to the original authors for their hard work and dedication. Without their contributions, this project would not have been possible.
+// I would like to express my gratitude to the original authors for their work.
 //
-// Original Implementation is here: https://github.com/remotely-save/remotely-save/blob/28b99557a864ef59c19d2ad96101196e401718f0/src/remoteForS3.ts
+// Original implementation: https://github.com/remotely-save/remotely-save/blob/28b99557a864ef59c19d2ad96101196e401718f0/src/remoteForS3.ts
 
 import { FetchHttpHandler, type FetchHttpHandlerOptions } from "@smithy/fetch-http-handler";
 import { HttpRequest, HttpResponse } from "@smithy/protocol-http";
@@ -102,6 +102,7 @@ export class ObsHttpHandler extends FetchHttpHandler {
             method: method,
             url: url,
             contentType: contentType,
+            throw: false,
         };
 
         const raceOfPromises = [

src/modules/essentialObsidian/APILib/ObsHttpHandler.unit.spec.ts

@@ -1,9 +1,10 @@
+import { GetObjectCommand, S3Client } from "@aws-sdk/client-s3";
 import { HttpRequest } from "@smithy/protocol-http";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
 const requestUrlMock = vi.hoisted(() =>
     vi.fn<
-        (param: { body?: string | ArrayBuffer }) => Promise<{
+        (param: { body?: string | ArrayBuffer; throw?: boolean }) => Promise<{
             headers: Record<string, string>;
             status: number;
             arrayBuffer: ArrayBuffer;
@@ -28,6 +29,42 @@ function requestWithBody(body: unknown) {
     });
 }
 
+function mockS3ErrorResponse(status: number, code?: string) {
+    requestUrlMock.mockImplementation(async (param) => {
+        if (param.throw !== false) {
+            throw new Error(`Request failed, status ${status}`);
+        }
+        return {
+            headers: { "content-type": "application/xml" },
+            status,
+            arrayBuffer: new TextEncoder().encode(code ? `<Error><Code>${code}</Code></Error>` : "").buffer,
+        };
+    });
+}
+
+function createS3Client() {
+    return new S3Client({
+        region: "us-east-1",
+        credentials: {
+            accessKeyId: "access-key",
+            secretAccessKey: "secret-key",
+        },
+        endpoint: "https://objects.example.com",
+        forcePathStyle: true,
+        maxAttempts: 1,
+        requestHandler: new ObsHttpHandler(),
+    });
+}
+
+function getMissingObject(client: S3Client) {
+    return client.send(
+        new GetObjectCommand({
+            Bucket: "bucket",
+            Key: "missing.json",
+        })
+    );
+}
+
 describe("ObsHttpHandler request bodies", () => {
     beforeEach(() => {
         requestUrlMock.mockReset();
@@ -58,3 +95,56 @@ describe("ObsHttpHandler request bodies", () => {
         expect(requestUrlMock).not.toHaveBeenCalled();
     });
 });
+
+describe("ObsHttpHandler response handling", () => {
+    beforeEach(() => {
+        requestUrlMock.mockReset();
+    });
+
+    it("returns an HTTP error response to the Smithy client", async () => {
+        mockS3ErrorResponse(404, "NoSuchKey");
+        const request = new HttpRequest({
+            protocol: "https:",
+            hostname: "objects.example.com",
+            method: "GET",
+            path: "/bucket/missing.json",
+            headers: {},
+        });
+
+        const result = await new ObsHttpHandler().handle(request);
+
+        expect(requestUrlMock).toHaveBeenCalledWith(expect.objectContaining({ throw: false }));
+        expect(result.response.statusCode).toBe(404);
+    });
+
+    it.each([
+        { code: "NoSuchKey", name: "NoSuchKey" },
+        { code: undefined, name: "NotFound" },
+    ])("lets the S3 client classify a missing object as $name", async ({ code, name }) => {
+        mockS3ErrorResponse(404, code);
+
+        await expect(getMissingObject(createS3Client())).rejects.toMatchObject({
+            name,
+            $metadata: { httpStatusCode: 404 },
+        });
+    });
+
+    it.each([
+        { status: 403, code: "AccessDenied" },
+        { status: 500, code: "InternalError" },
+    ])("keeps an S3 $status response distinct from a missing object", async ({ status, code }) => {
+        mockS3ErrorResponse(status, code);
+
+        await expect(getMissingObject(createS3Client())).rejects.toMatchObject({
+            name: code,
+            $metadata: { httpStatusCode: status },
+        });
+    });
+
+    it("preserves a transport failure", async () => {
+        const failure = new Error("network failed");
+        requestUrlMock.mockRejectedValue(failure);
+
+        await expect(new ObsHttpHandler().handle(requestWithBody(new ArrayBuffer(0)))).rejects.toBe(failure);
+    });
+});

test/e2e-obsidian/runner/setupUri.ts

@@ -19,6 +19,7 @@ export type SetupState = {
     endpoint: string;
     bucket: string;
     bucketPrefix: string;
+    useCustomRequestHandler: boolean;
     p2pEnabled: boolean;
     p2pRelays: string;
     p2pRoomId: string;
@@ -354,6 +355,7 @@ export async function readSetupState(cliBinary: string, environment: NodeJS.Proc
             "endpoint:settings.endpoint||'',",
             "bucket:settings.bucket||'',",
             "bucketPrefix:settings.bucketPrefix||'',",
+            "useCustomRequestHandler:settings.useCustomRequestHandler===true,",
             "p2pEnabled:settings.P2P_Enabled===true,",
             "p2pRelays:settings.P2P_relays||'',",
             "p2pRoomId:settings.P2P_roomID||'',",

test/e2e-obsidian/scripts/local-suite.ts

@@ -35,6 +35,10 @@ const testSteps: Step[] = [
         name: "Object Storage Setup URI workflow",
         args: ["run", "test:e2e:obsidian:object-storage-setup-uri-workflow"],
     },
+    {
+        name: "Object Storage Custom HTTP Handler Setup URI workflow",
+        args: ["run", "test:e2e:obsidian:object-storage-custom-http-handler-setup-uri-workflow"],
+    },
     { name: "P2P Setup URI workflow", args: ["run", "test:e2e:obsidian:p2p-setup-uri-workflow"] },
     { name: "startup scan", args: ["run", "test:e2e:obsidian:startup-scan"] },
     { name: "provisioned Setup URI workflow", args: ["run", "test:e2e:obsidian:setup-uri-workflow"] },

test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts

@@ -45,7 +45,10 @@ import { createTemporaryVault, type TemporaryVault } from "../runner/vault.ts";
 process.env.E2E_OBSIDIAN_CLI_TIMEOUT_MS ??= "90000";
 
 const execFileAsync = promisify(execFile);
-const captures: SetupCaptureNames = { scenario: "object-storage-setup-uri", guide: "object-storage-setup" };
+const useCustomRequestHandler = process.argv.includes("--custom-http-handler");
+const captures: SetupCaptureNames = useCustomRequestHandler
+    ? { scenario: "object-storage-custom-http-handler-setup-uri", guide: "object-storage-custom-http-handler-setup" }
+    : { scenario: "object-storage-setup-uri", guide: "object-storage-setup" };
 const noteFromFirst = "E2E/object-storage/from-first.md";
 const noteFromSecond = "E2E/object-storage/from-second.md";
 const firstContent =
@@ -94,7 +97,8 @@ async function runDeno(script: string, environment: NodeJS.ProcessEnv): Promise<
 
 async function generateBootstrapSetupURI(
     objectStorage: ObjectStorageConfig,
-    bucketPrefix: string
+    bucketPrefix: string,
+    useCustomRequestHandler: boolean
 ): Promise<SetupArtifact> {
     const setupPassphrase = randomBytes(24).toString("base64url");
     const output = await runDeno("utils/setup/generate_setup_uri.ts", {
@@ -107,6 +111,7 @@ async function generateBootstrapSetupURI(
         region: objectStorage.region,
         force_path_style: String(objectStorage.forcePathStyle),
         bucket_prefix: bucketPrefix,
+        ...(useCustomRequestHandler ? { use_custom_request_handler: "true" } : {}),
         passphrase: randomBytes(24).toString("base64url"),
         uri_passphrase: setupPassphrase,
     });
@@ -251,7 +256,7 @@ async function main(): Promise<void> {
 
     const objectStorage = await loadObjectStorageConfig();
     const bucketPrefix = makeUniqueBucketPrefix("setup-uri-workflow");
-    const bootstrapArtifact = await generateBootstrapSetupURI(objectStorage, bucketPrefix);
+    const bootstrapArtifact = await generateBootstrapSetupURI(objectStorage, bucketPrefix, useCustomRequestHandler);
     const vaultA = await createTemporaryVault();
     const vaultB = await createTemporaryVault();
     const [portA, portB] = sessionPorts();
@@ -285,6 +290,11 @@ async function main(): Promise<void> {
             bucketPrefix,
             "The first device did not activate the unique bucket prefix."
         );
+        assertEqual(
+            firstState.useCustomRequestHandler,
+            useCustomRequestHandler,
+            "The first device did not preserve the expected Custom HTTP Handler setting."
+        );
 
         await writeNote(context.cliBinary, sessionA.cliEnv, noteFromFirst, firstContent);
         await pushLocalChanges(context.cliBinary, sessionA.cliEnv);
@@ -330,6 +340,11 @@ async function main(): Promise<void> {
             bucketPrefix,
             "The second device did not import the unique bucket prefix."
         );
+        assertEqual(
+            secondState.useCustomRequestHandler,
+            useCustomRequestHandler,
+            "The second device did not import the expected Custom HTTP Handler setting."
+        );
         await pushLocalChanges(context.cliBinary, sessionB.cliEnv);
         await waitForPathContent(vaultB, noteFromFirst, firstContent);
         screenshots.push(
@@ -337,7 +352,7 @@ async function main(): Promise<void> {
                 portB,
                 noteFromFirst,
                 "Object Storage from the first device",
-                "guide-object-storage-setup-first-to-second.png"
+                `guide-${captures.guide}-first-to-second.png`
             )
         );
 
@@ -357,12 +372,14 @@ async function main(): Promise<void> {
                 portA,
                 noteFromSecond,
                 "Object Storage from the second device",
-                "guide-object-storage-setup-second-to-first.png"
+                `guide-${captures.guide}-second-to-first.png`
             )
         );
 
         console.log(
-            `Object Storage Setup URI and two-device roundtrip succeeded. Screenshots: ${screenshots.join(", ")}`
+            `Object Storage Setup URI and two-device roundtrip succeeded with the ${
+                useCustomRequestHandler ? "Custom HTTP Handler" : "default HTTP handler"
+            }. Screenshots: ${screenshots.join(", ")}`
         );
     } finally {
         await stopSessions(context).catch((error: unknown) => {

test/e2e-obsidian/scripts/run-focused.ts

@@ -21,6 +21,7 @@ const focusedScenarios = new Set([
     "cli-to-obsidian-sync",
     "minio-upload",
     "object-storage-setup-uri-workflow",
+    "object-storage-custom-http-handler-setup-uri-workflow",
     "p2p-setup-uri-workflow",
     "partial-startup-file-failure",
     "startup-scan",

updates.md

@@ -12,6 +12,12 @@ Earlier releases remain available in the 1.0 release history, the 1.0 preview hi
 
 ## Unreleased
 
+### Synchronisation and storage
+
+#### Fixed
+
+- First-time Object Storage setup now completes when **Use Custom HTTP Handler** is enabled for an empty remote, including a new Cloudflare R2 bucket. LiveSync can now create the remote state required to begin synchronisation. (#1166)
+
 ## 1.0.26
 
 ~~1.0.25~~ was cancelled because pre-release validation found that LiveSync could appear to finish synchronising even though Android had not written a received file to the Vault; the warning appeared only after restart.