diff --git a/package.json b/package.json index b8f35f2b..5cb77f80 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,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", diff --git a/src/modules/essentialObsidian/APILib/ObsHttpHandler.ts b/src/modules/essentialObsidian/APILib/ObsHttpHandler.ts index bc7082ea..bd480dc0 100644 --- a/src/modules/essentialObsidian/APILib/ObsHttpHandler.ts +++ b/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. -// 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. +// This file was originally based on code published by @remotely-save under the Apache License 2.0. +// 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 = [ diff --git a/src/modules/essentialObsidian/APILib/ObsHttpHandler.unit.spec.ts b/src/modules/essentialObsidian/APILib/ObsHttpHandler.unit.spec.ts index 110f78db..48291dfb 100644 --- a/src/modules/essentialObsidian/APILib/ObsHttpHandler.unit.spec.ts +++ b/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; 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 ? `${code}` : "").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); + }); +}); diff --git a/test/e2e-obsidian/runner/setupUri.ts b/test/e2e-obsidian/runner/setupUri.ts index 4f292bc0..ffd9fb1d 100644 --- a/test/e2e-obsidian/runner/setupUri.ts +++ b/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||'',", diff --git a/test/e2e-obsidian/scripts/local-suite.ts b/test/e2e-obsidian/scripts/local-suite.ts index 9cb9404f..87da7263 100644 --- a/test/e2e-obsidian/scripts/local-suite.ts +++ b/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"] }, diff --git a/test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts b/test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts index 87fd70fd..9316e4b8 100644 --- a/test/e2e-obsidian/scripts/object-storage-setup-uri-workflow.ts +++ b/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 { 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 { 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 { 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 { 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 { 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 { 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) => { diff --git a/test/e2e-obsidian/scripts/run-focused.ts b/test/e2e-obsidian/scripts/run-focused.ts index f5697217..508f258c 100644 --- a/test/e2e-obsidian/scripts/run-focused.ts +++ b/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", diff --git a/updates.md b/updates.md index 368a4e66..9a7fa637 100644 --- a/updates.md +++ b/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.