From 59ab44cd58c8534a15be98f8271994460e439158 Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Sun, 16 Aug 2026 15:13:40 -0500 Subject: [PATCH] Fall back to PDS for current user and make plugins non-blocking --- package.json | 2 +- src/js/app.js | 6 +- src/js/atproto.js | 26 +++++- src/js/dataHelpers.js | 25 ++++++ src/js/dataLayer/declarative.js | 2 +- src/js/dataLayer/requests.js | 31 ++++++- src/js/plugins/pluginService.js | 33 +++++++- src/js/templates/sidebar.template.js | 83 +++++++++++-------- tests/unit/specs/atproto.test.js | 21 ++++- tests/unit/specs/dataHelpers.test.js | 58 +++++++++++++ tests/unit/specs/dataLayer/requests.test.js | 66 +++++++++++++++ .../unit/specs/plugins/pluginService.test.js | 21 ++++- .../specs/templates/sidebar.template.test.js | 17 ++++ 13 files changed, 341 insertions(+), 50 deletions(-) diff --git a/package.json b/package.json index b5b8e01c..0da53adc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "impro", - "version": "0.18.210", + "version": "0.18.211", "type": "module", "scripts": { "start": "rm -rf \"${BUILD_DIR:-build}\" && NODE_ENV=development eleventy --serve", diff --git a/src/js/app.js b/src/js/app.js index 045cecf4..d70c1668 100644 --- a/src/js/app.js +++ b/src/js/app.js @@ -198,11 +198,9 @@ export async function main() { ); } - try { - await pluginService.loadEnabledPlugins(); - } catch (error) { + pluginService.loadEnabledPlugins().catch((error) => { console.error("Error loading plugins", error); - } + }); if (notificationService) { notificationService.startPolling(); diff --git a/src/js/atproto.js b/src/js/atproto.js index b82c492d..95c7201c 100644 --- a/src/js/atproto.js +++ b/src/js/atproto.js @@ -21,14 +21,34 @@ export function didDocReferencesHandle(didDoc, handle) { return aliases.includes(atHandle); } +const RESOLVE_HANDLE_TIMEOUT_MS = 5000; + export async function resolveHandle(handle) { const params = new URLSearchParams({ handle, }); - const res = await fetch( - `${HANDLE_RESOLVER_SERVICE_URL}/xrpc/com.atproto.identity.resolveHandle?` + - params.toString(), + const controller = new AbortController(); + const timeoutId = setTimeout( + () => controller.abort(), + RESOLVE_HANDLE_TIMEOUT_MS, ); + let res; + try { + res = await fetch( + `${HANDLE_RESOLVER_SERVICE_URL}/xrpc/com.atproto.identity.resolveHandle?` + + params.toString(), + { signal: controller.signal }, + ); + } catch (error) { + if (controller.signal.aborted) { + throw new Error( + `resolveHandle: timed out after ${RESOLVE_HANDLE_TIMEOUT_MS}ms resolving "${handle}"`, + ); + } + throw error; + } finally { + clearTimeout(timeoutId); + } const data = await res.json(); return data.did ?? null; } diff --git a/src/js/dataHelpers.js b/src/js/dataHelpers.js index 8894afb5..f320191d 100644 --- a/src/js/dataHelpers.js +++ b/src/js/dataHelpers.js @@ -35,6 +35,31 @@ export function buildCdnUrl(prefix, did, cid) { return `${BSKY_CDN_URL}/img/${prefix}/plain/${did}/${cid}@jpeg`; } +function blobCdnUrl(prefix, did, blob) { + const cid = blob?.ref?.$link ?? null; + if (!cid) return null; + return buildCdnUrl(prefix, did, cid); +} + +// A profileViewDetailed-shaped object assembled from the raw +// app.bsky.actor.profile record, for when the appview is unreachable. +// Appview-computed fields (counts, viewer, labels, associated) are absent. +export function buildProfileFromRecord({ did, handle, record }) { + const value = record?.value ?? {}; + return { + did, + handle, + displayName: value.displayName ?? null, + description: value.description ?? null, + avatar: blobCdnUrl("avatar", did, value.avatar), + banner: blobCdnUrl("banner", did, value.banner), + pinnedPost: value.pinnedPost ?? null, + createdAt: value.createdAt ?? null, + labels: [], + isPartial: true, + }; +} + export function avatarThumbnailUrl(avatarUrl) { if (!avatarUrl) { console.warn("avatarUrl is null"); diff --git a/src/js/dataLayer/declarative.js b/src/js/dataLayer/declarative.js index 8d359abb..8324dec0 100644 --- a/src/js/dataLayer/declarative.js +++ b/src/js/dataLayer/declarative.js @@ -5,7 +5,7 @@ export class Declarative { } async ensureCurrentUser() { let currentUser = this.derived.$currentUser.get(); - if (!currentUser) { + if (!currentUser || currentUser.isPartial) { await this.requests.loadCurrentUser(); currentUser = this.derived.$currentUser.get(); } diff --git a/src/js/dataLayer/requests.js b/src/js/dataLayer/requests.js index 724df900..529c1760 100644 --- a/src/js/dataLayer/requests.js +++ b/src/js/dataLayer/requests.js @@ -15,6 +15,7 @@ import { getJoinLinkCodesFromMessages, getPostsFromPostThread, getPostsFromFeed, + buildProfileFromRecord, } from "/js/dataHelpers.js"; import { getLocalRefsFromDraft } from "/js/dataHelpers.js"; import { unique } from "/js/utils.js"; @@ -266,10 +267,38 @@ export class Requests { async loadCurrentUser() { const session = await this.api.getSession(); - const profile = await this.api.getProfile(session.did); + let profile; + try { + profile = await this.api.getProfile(session.did); + } catch (error) { + console.warn( + "getProfile failed, falling back to the profile record", + error, + ); + profile = await this.loadCurrentUserFromRecord(session); + } this.dataStore.$currentUser.set(profile); } + async loadCurrentUserFromRecord(session) { + let record = null; + try { + record = await this.api.getProfileRecord(); + } catch (error) { + if ( + !(error instanceof ApiError) || + error.data?.error !== "RecordNotFound" + ) { + throw error; + } + } + return buildProfileFromRecord({ + did: session.did, + handle: session.handle, + record, + }); + } + async loadPostThread(postURI, { depth = 6 } = {}) { const labelers = this.requireLabelers(); let [postThread, postThreadOther] = await Promise.all([ diff --git a/src/js/plugins/pluginService.js b/src/js/plugins/pluginService.js index 7263f2e4..a7a62faf 100644 --- a/src/js/plugins/pluginService.js +++ b/src/js/plugins/pluginService.js @@ -37,13 +37,21 @@ import { normalizeFetchOrigin, } from "/js/plugins/pluginPermissions.js"; import { compareVersions, groupBy, isDev, sortBy } from "/js/utils.js"; -import { Signal, SignalMap, SignalSet, ReactiveStore } from "/js/signals.js"; +import { + Signal, + SignalMap, + SignalSet, + ReactiveStore, + untrack, +} from "/js/signals.js"; import { EventEmitter } from "/js/eventEmitter.js"; import { PLUGIN_REGISTRY_URL } from "/js/config.js"; import { getFeedGeneratorProxyUrl } from "/js/dataHelpers.js"; const DISABLE_PLUGINS_QUERY_PARAM = "disable-plugins"; +const INITIAL_PLUGIN_LOAD_TIMEOUT_MS = 3000; + function requireHostMethodArg(method, name, value) { if (!value) { throw new Error(`${method} requires a ${name}`); @@ -195,6 +203,9 @@ export class PluginService extends ReactiveStore { // Keyed by `${pluginId}:${pageId}` — each plugin can register multiple pages this.$pages = new SignalMap(); this.$initialLoadComplete = new Signal.State(false); + this._initialLoadPromise = new Promise((resolve) => { + this._resolveInitialLoad = resolve; + }); this.slotDispatcher = new PluginSlotDispatcher(); this.richTextDispatcher = new PluginRichTextDispatcher({ getRenderer: (pluginId) => this.getRenderer(pluginId), @@ -229,8 +240,21 @@ export class PluginService extends ReactiveStore { this._setupFeedFilterIntegration(); } + async _waitForInitialPluginLoad() { + if (untrack(() => this.$initialLoadComplete.get())) return; + let timeoutId = null; + await Promise.race([ + this._initialLoadPromise, + new Promise((resolve) => { + timeoutId = setTimeout(resolve, INITIAL_PLUGIN_LOAD_TIMEOUT_MS); + }), + ]); + clearTimeout(timeoutId); + } + _setupFeedFilterIntegration() { this._dataLayer.on("feedLoaded", async ({ feedURI, feed, reload }) => { + await this._waitForInitialPluginLoad(); const overrides = await this.getFilteredFeedItems(feedURI, feed); if (reload) { this._hiddenFeedItemsStore.replace(feedURI, overrides); @@ -756,10 +780,15 @@ export class PluginService extends ReactiveStore { try { await this._loadEnabledPlugins(); } finally { - this.$initialLoadComplete.set(true); + this._completeInitialLoad(); } } + _completeInitialLoad() { + this.$initialLoadComplete.set(true); + this._resolveInitialLoad(); + } + async _loadEnabledPlugins() { if (arePluginsDisabledByQueryParam()) { const enabledPluginIds = this.prefManager.$enabledPlugins diff --git a/src/js/templates/sidebar.template.js b/src/js/templates/sidebar.template.js index a2c5d642..3c35cd03 100644 --- a/src/js/templates/sidebar.template.js +++ b/src/js/templates/sidebar.template.js @@ -283,6 +283,8 @@ export function sidebarTemplate({ const handle = currentUser?.handle ? "@" + currentUser.handle : null; const followersCount = currentUser?.followersCount ?? null; const followsCount = currentUser?.followsCount ?? null; + // A partial profile has no counts to show + const showStats = !currentUser?.isPartial; const longPressEnabled = !!onLongPressProfile; return html` @@ -314,43 +316,52 @@ export function sidebarTemplate({ ${handle || html` `} - + + currentUser + ? navigateFromSidebar( + event, + linkToProfileFollowers(currentUser), + ) + : null} + > + ${followersCount !== null + ? formatLargeNumber(followersCount) + : ""} + followers + + · + + currentUser + ? navigateFromSidebar( + event, + linkToProfileFollowing(currentUser), + ) + : null} + > + ${followsCount !== null + ? formatLargeNumber(followsCount) + : ""} + following + + ` + : null} ${sidebarNavTemplate({ diff --git a/tests/unit/specs/atproto.test.js b/tests/unit/specs/atproto.test.js index abe25cab..d63cb6de 100644 --- a/tests/unit/specs/atproto.test.js +++ b/tests/unit/specs/atproto.test.js @@ -1,4 +1,4 @@ -import { describe, it, beforeEach, afterEach } from "node:test"; +import { describe, it, beforeEach, afterEach, mock } from "node:test"; import assert from "node:assert/strict"; import { resolveHandle, @@ -18,6 +18,7 @@ describe("atproto handle resolution", () => { afterEach(() => { globalThis.fetch = originalFetch; + mock.timers.reset(); }); function stubDid(did) { @@ -89,6 +90,24 @@ describe("atproto handle resolution", () => { stubDid(null); assert.deepEqual(await resolveHandle("nope.example"), null); }); + + it("throws when the resolver does not respond in time", async () => { + mock.timers.enable({ apis: ["setTimeout"] }); + globalThis.fetch.__intercept( + /resolveHandle/, + (url, options) => + new Promise((resolve, reject) => { + options.signal.addEventListener("abort", () => + reject(new Error("aborted")), + ); + }), + ); + + const resolving = resolveHandle("slow.example"); + mock.timers.tick(5000); + + await assert.rejects(resolving, /timed out/); + }); }); describe("getServiceEndpointForHandle", () => { diff --git a/tests/unit/specs/dataHelpers.test.js b/tests/unit/specs/dataHelpers.test.js index f88589f1..9637bf8f 100644 --- a/tests/unit/specs/dataHelpers.test.js +++ b/tests/unit/specs/dataHelpers.test.js @@ -2,6 +2,7 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; import { avatarThumbnailUrl, + buildProfileFromRecord, getRKey, getIsLiked, isListFeed, @@ -51,6 +52,63 @@ import { } from "/js/dataHelpers.js"; import { IN_APP_LINK_DOMAINS } from "/js/config.js"; +describe("buildProfileFromRecord", () => { + const did = "did:plc:me"; + const blob = (cid) => ({ + $type: "blob", + ref: { $link: cid }, + mimeType: "image/jpeg", + size: 1000, + }); + + it("should map record fields and build CDN urls for blobs", () => { + const profile = buildProfileFromRecord({ + did, + handle: "me.test", + record: { + uri: `at://${did}/app.bsky.actor.profile/self`, + value: { + displayName: "Me", + description: "hello", + avatar: blob("avatarcid"), + banner: blob("bannercid"), + pinnedPost: { uri: `at://${did}/app.bsky.feed.post/1`, cid: "abc" }, + createdAt: "2024-01-01T00:00:00.000Z", + }, + }, + }); + + assert.deepEqual(profile, { + did, + handle: "me.test", + displayName: "Me", + description: "hello", + avatar: `https://cdn.bsky.app/img/avatar/plain/${did}/avatarcid@jpeg`, + banner: `https://cdn.bsky.app/img/banner/plain/${did}/bannercid@jpeg`, + pinnedPost: { uri: `at://${did}/app.bsky.feed.post/1`, cid: "abc" }, + createdAt: "2024-01-01T00:00:00.000Z", + labels: [], + isPartial: true, + }); + }); + + it("should null out missing fields when there is no record", () => { + const profile = buildProfileFromRecord({ + did, + handle: "me.test", + record: null, + }); + + assert.deepEqual(profile.did, did); + assert.deepEqual(profile.handle, "me.test"); + assert.deepEqual(profile.displayName, null); + assert.deepEqual(profile.avatar, null); + assert.deepEqual(profile.banner, null); + assert.deepEqual(profile.pinnedPost, null); + assert.deepEqual(profile.isPartial, true); + }); +}); + describe("avatarThumbnailUrl", () => { it("should convert plain avatar URL to thumbnail URL", () => { const avatarUrl = diff --git a/tests/unit/specs/dataLayer/requests.test.js b/tests/unit/specs/dataLayer/requests.test.js index 932a7bb3..e93bbc0b 100644 --- a/tests/unit/specs/dataLayer/requests.test.js +++ b/tests/unit/specs/dataLayer/requests.test.js @@ -3322,6 +3322,72 @@ describe("loadCurrentUser", () => { assert.deepEqual(requestedDid, "did:plc:me"); assert.deepEqual(dataStore.$currentUser.get(), profile); }); + + it("should fall back to the profile record when getProfile fails", async () => { + const mockApi = { + getSession: async () => ({ did: "did:plc:me", handle: "me.test" }), + getProfile: async () => { + throw new ApiError({ status: 502, statusText: "Bad Gateway" }); + }, + getProfileRecord: async () => ({ + uri: "at://did:plc:me/app.bsky.actor.profile/self", + value: { displayName: "Me" }, + }), + }; + const dataStore = new DataStore(); + const requests = makeRequests(mockApi, dataStore); + + await requests.loadCurrentUser(); + + const currentUser = dataStore.$currentUser.get(); + assert.deepEqual(currentUser.did, "did:plc:me"); + assert.deepEqual(currentUser.handle, "me.test"); + assert.deepEqual(currentUser.displayName, "Me"); + assert.deepEqual(currentUser.isPartial, true); + }); + + it("should fall back to a record-less profile when the user has no profile record", async () => { + const mockApi = { + getSession: async () => ({ did: "did:plc:me", handle: "me.test" }), + getProfile: async () => { + throw new ApiError({ status: 502, statusText: "Bad Gateway" }); + }, + getProfileRecord: async () => { + throw new ApiError({ + status: 400, + statusText: "Bad Request", + data: { error: "RecordNotFound" }, + }); + }, + }; + const dataStore = new DataStore(); + const requests = makeRequests(mockApi, dataStore); + + await requests.loadCurrentUser(); + + const currentUser = dataStore.$currentUser.get(); + assert.deepEqual(currentUser.handle, "me.test"); + assert.deepEqual(currentUser.displayName, null); + assert.deepEqual(currentUser.isPartial, true); + }); + + it("should rethrow when the profile record request fails for another reason", async () => { + const recordError = new ApiError({ status: 500, statusText: "Oops" }); + const mockApi = { + getSession: async () => ({ did: "did:plc:me", handle: "me.test" }), + getProfile: async () => { + throw new ApiError({ status: 502, statusText: "Bad Gateway" }); + }, + getProfileRecord: async () => { + throw recordError; + }, + }; + const dataStore = new DataStore(); + const requests = makeRequests(mockApi, dataStore); + + await assert.rejects(() => requests.loadCurrentUser(), recordError); + assert.deepEqual(dataStore.$currentUser.get(), null); + }); }); describe("loadPost", () => { diff --git a/tests/unit/specs/plugins/pluginService.test.js b/tests/unit/specs/plugins/pluginService.test.js index 4f34b34b..5c85c15b 100644 --- a/tests/unit/specs/plugins/pluginService.test.js +++ b/tests/unit/specs/plugins/pluginService.test.js @@ -1321,7 +1321,10 @@ describe("getFilteredFeedItems", () => { }); describe("feed filter integration", () => { - function makeHarness(getFilteredFeedItems) { + function makeHarness( + getFilteredFeedItems, + { initialLoadComplete = true } = {}, + ) { const dataLayer = emptyDataLayer(); const hiddenFeedItemsStore = new HiddenFeedItemsStore(); const service = makeServiceWithRealBridge({ @@ -1329,6 +1332,7 @@ describe("feed filter integration", () => { hiddenFeedItemsStore, }); service.getFilteredFeedItems = getFilteredFeedItems; + if (initialLoadComplete) service._completeInitialLoad(); return { service, dataLayer, hiddenFeedItemsStore }; } @@ -1349,6 +1353,21 @@ describe("feed filter integration", () => { assert.deepEqual(hiddenFeedItemsStore.get("f"), { p1: false, p2: false }); }); + it("defers filtering a page until the initial plugin load completes", async () => { + const { service, dataLayer, hiddenFeedItemsStore } = makeHarness( + async () => ({ p1: false }), + { initialLoadComplete: false }, + ); + + dataLayer.emit("feedLoaded", { feedURI: "f", feed: {}, reload: false }); + await flush(); + assert.deepEqual(hiddenFeedItemsStore.get("f"), {}); + + service._completeInitialLoad(); + await flush(); + assert.deepEqual(hiddenFeedItemsStore.get("f"), { p1: false }); + }); + it("replaces on reload", async () => { let call = 0; const { dataLayer, hiddenFeedItemsStore } = makeHarness(async () => { diff --git a/tests/unit/specs/templates/sidebar.template.test.js b/tests/unit/specs/templates/sidebar.template.test.js index 72d9d7fd..d260f45b 100644 --- a/tests/unit/specs/templates/sidebar.template.test.js +++ b/tests/unit/specs/templates/sidebar.template.test.js @@ -201,6 +201,23 @@ describe("sidebarTemplate - logged in state", () => { assert(stats.textContent.includes("50")); assert(stats.textContent.includes("following")); }); + + it("should omit the stats row for a partial profile", () => { + const result = sidebarTemplate({ + isAuthenticated: true, + currentUser: { ...mockUser, isPartial: true }, + }); + const container = document.createElement("div"); + render(result, container); + assert.deepEqual( + container.querySelector("[data-testid='sidebar-profile-stats']"), + null, + ); + const handle = container.querySelector( + "[data-testid='sidebar-profile-handle']", + ); + assert(handle !== null); + }); }); describe("sidebarTemplate - nav items", () => { -- 2.51.2