Skip to content

Commit 1daa887

Browse files
manztpre-commit-ci[bot]mscolnick
authored
Fix anywidget initial buffer handling regression (#6520)
Fixes #6499 The `msgspec` migration changed how we encode "buffers" (aka binary blobs) in the `UIElement.text`, causing our frontend to dual-encode the responses. This commit properly decodes buffers at system boundaries by converting base64 strings to `DataView` objects at two key points: - When extracting buffers from `SendUIElementMessage` operations in the WebSocket handler - When resolving the initial value from HTML attributes in the AnyWidgetPlugin component I added several tests were we missed these changes, verifying that buffers are correctly encoded as base64 in HTML and properly handled in the Python backend. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Myles Scolnick <[email protected]>
1 parent 6182a6d commit 1daa887

File tree

20 files changed

+298
-134
lines changed

20 files changed

+298
-134
lines changed

frontend/src/core/dom/events.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const MarimoIncomingMessageEvent = defineCustomEvent(
4343
)<{
4444
objectId: UIElementId;
4545
message: unknown;
46-
buffers: DataView[] | undefined;
46+
buffers: readonly DataView[];
4747
}>();
4848
export type MarimoIncomingMessageEventType = ReturnType<
4949
typeof MarimoIncomingMessageEvent.create

frontend/src/core/dom/uiregistry.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

3-
import { byteStringToDataView } from "@/utils/data-views";
4-
import type { Base64String } from "@/utils/json/base64";
5-
import { typedAtob } from "@/utils/json/base64";
63
import { Logger } from "@/utils/Logger";
74
import type { CellId, UIElementId } from "../cells/ids";
85
import {
@@ -136,15 +133,12 @@ export class UIElementRegistry {
136133
broadcastMessage(
137134
objectId: UIElementId,
138135
message: unknown,
139-
buffers: Base64String[] | undefined | null,
136+
buffers: readonly DataView[],
140137
): void {
141138
const entry = this.entries.get(objectId);
142139
if (entry === undefined) {
143140
Logger.warn("UIElementRegistry missing entry", objectId);
144141
} else {
145-
const toDataView = (base64: Base64String) => {
146-
return byteStringToDataView(typedAtob(base64));
147-
};
148142
entry.elements.forEach((element) => {
149143
element.dispatchEvent(
150144
MarimoIncomingMessageEvent.create({
@@ -153,7 +147,7 @@ export class UIElementRegistry {
153147
detail: {
154148
objectId: objectId,
155149
message: message,
156-
buffers: buffers ? buffers.map(toDataView) : undefined,
150+
buffers: buffers,
157151
},
158152
}),
159153
);

frontend/src/core/islands/main.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { renderHTML } from "@/plugins/core/RenderHTML";
1616
import { initializePlugins } from "@/plugins/plugins";
1717
import { logNever } from "@/utils/assertNever";
1818
import { Functions } from "@/utils/functions";
19-
import type { Base64String } from "@/utils/json/base64";
19+
import { safeExtractSetUIElementMessageBuffers } from "@/utils/json/base64";
2020
import { jsonParseWithSpecialChar } from "@/utils/json/json-parser";
2121
import { Logger } from "@/utils/Logger";
2222
import {
@@ -145,7 +145,7 @@ export async function initialize() {
145145
UI_ELEMENT_REGISTRY.broadcastMessage(
146146
msg.data.ui_element as UIElementId,
147147
msg.data.message,
148-
msg.data.buffers as Base64String[],
148+
safeExtractSetUIElementMessageBuffers(msg.data),
149149
);
150150
return;
151151

frontend/src/core/websocket/useMarimoWebSocket.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
} from "@/plugins/impl/anywidget/model";
1717
import { logNever } from "@/utils/assertNever";
1818
import { prettyError } from "@/utils/errors";
19-
import type { Base64String, JsonString } from "@/utils/json/base64";
19+
import {
20+
type JsonString,
21+
safeExtractSetUIElementMessageBuffers,
22+
} from "@/utils/json/base64";
2023
import { jsonParseWithSpecialChar } from "@/utils/json/json-parser";
2124
import { Logger } from "@/utils/Logger";
2225
import { reloadSafe } from "@/utils/reload-safe";
@@ -112,7 +115,7 @@ export function useMarimoWebSocket(opts: {
112115
const modelId = msg.data.model_id;
113116
const uiElement = msg.data.ui_element;
114117
const message = msg.data.message;
115-
const buffers = (msg.data.buffers ?? []) as Base64String[];
118+
const buffers = safeExtractSetUIElementMessageBuffers(msg.data);
116119

117120
if (modelId && isMessageWidgetState(message)) {
118121
handleWidgetMessage({

frontend/src/plugins/impl/anywidget/AnyWidgetPlugin.tsx

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
/* eslint-disable @typescript-eslint/no-explicit-any */
33

44
import type { AnyWidget, Experimental } from "@anywidget/types";
5-
import { isEqual } from "lodash-es";
5+
import { get, isEqual, set } from "lodash-es";
66
import { useEffect, useMemo, useRef } from "react";
77
import { z } from "zod";
88
import { MarimoIncomingMessageEvent } from "@/core/dom/events";
@@ -16,7 +16,11 @@ import {
1616
import { createPlugin } from "@/plugins/core/builder";
1717
import { rpc } from "@/plugins/core/rpc";
1818
import type { IPluginProps } from "@/plugins/types";
19-
import { updateBufferPaths } from "@/utils/data-views";
19+
import {
20+
type Base64String,
21+
byteStringToBinary,
22+
typedAtob,
23+
} from "@/utils/json/base64";
2024
import { Logger } from "@/utils/Logger";
2125
import { ErrorBanner } from "../common/error-banner";
2226
import { MODEL_MANAGER, Model } from "./model";
@@ -59,6 +63,11 @@ type Props = IPluginProps<T, Data, PluginFunctions>;
5963

6064
const AnyWidgetSlot = (props: Props) => {
6165
const { css, jsUrl, jsHash, bufferPaths } = props.data;
66+
67+
const valueWithBuffers = useMemo(() => {
68+
return resolveInitialValue(props.value, bufferPaths ?? []);
69+
}, [props.value, bufferPaths]);
70+
6271
// JS is an ESM file with a render function on it
6372
// export function render({ model, el }) {
6473
// ...
@@ -85,10 +94,6 @@ const AnyWidgetSlot = (props: Props) => {
8594
}
8695
}, [hasError, jsUrl]);
8796

88-
const valueWithBuffer = useMemo(() => {
89-
return updateBufferPaths(props.value, bufferPaths);
90-
}, [props.value, bufferPaths]);
91-
9297
// Mount the CSS
9398
useEffect(() => {
9499
const shadowRoot = props.host.shadowRoot;
@@ -157,7 +162,7 @@ const AnyWidgetSlot = (props: Props) => {
157162
key={key}
158163
{...props}
159164
widget={module.default}
160-
value={valueWithBuffer}
165+
value={valueWithBuffers}
161166
/>
162167
);
163168
};
@@ -284,3 +289,16 @@ export const visibleForTesting = {
284289
isAnyWidgetModule,
285290
getDirtyFields,
286291
};
292+
293+
export function resolveInitialValue(
294+
raw: Record<string, any>,
295+
bufferPaths: ReadonlyArray<ReadonlyArray<string | number>>,
296+
) {
297+
const out = structuredClone(raw);
298+
for (const bufferPath of bufferPaths) {
299+
const base64String: Base64String = get(raw, bufferPath);
300+
const bytes = byteStringToBinary(typedAtob(base64String));
301+
set(out, bufferPath, new DataView(bytes.buffer));
302+
}
303+
return out;
304+
}

frontend/src/plugins/impl/anywidget/__tests__/AnyWidgetPlugin.test.tsx

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
55
import { TestUtils } from "@/__tests__/test-helpers";
66
import type { UIElementId } from "@/core/cells/ids";
77
import { MarimoIncomingMessageEvent } from "@/core/dom/events";
8-
import { getDirtyFields, visibleForTesting } from "../AnyWidgetPlugin";
8+
import {
9+
getDirtyFields,
10+
resolveInitialValue,
11+
visibleForTesting,
12+
} from "../AnyWidgetPlugin";
913
import { Model } from "../model";
1014

1115
const { LoadedSlot } = visibleForTesting;
@@ -129,7 +133,7 @@ describe("LoadedSlot", () => {
129133
method: "update",
130134
state: { count: 10 },
131135
},
132-
buffers: undefined,
136+
buffers: [],
133137
},
134138
bubbles: false,
135139
composed: true,
@@ -179,3 +183,55 @@ describe("LoadedSlot", () => {
179183
});
180184
});
181185
});
186+
187+
describe("resolveInitialValue", () => {
188+
it("should convert base64 strings to DataView at specified paths", () => {
189+
const result = resolveInitialValue(
190+
{
191+
a: 10,
192+
b: "aGVsbG8=", // "hello" in base64
193+
c: [1, "d29ybGQ="], // "world" in base64
194+
d: {
195+
foo: "bWFyaW1vCg==", // "marimo" in base64
196+
baz: 20,
197+
},
198+
},
199+
[["b"], ["c", 1], ["d", "foo"]],
200+
);
201+
202+
expect(result).toMatchInlineSnapshot(`
203+
{
204+
"a": 10,
205+
"b": DataView [
206+
104,
207+
101,
208+
108,
209+
108,
210+
111,
211+
],
212+
"c": [
213+
1,
214+
DataView [
215+
119,
216+
111,
217+
114,
218+
108,
219+
100,
220+
],
221+
],
222+
"d": {
223+
"baz": 20,
224+
"foo": DataView [
225+
109,
226+
97,
227+
114,
228+
105,
229+
109,
230+
111,
231+
10,
232+
],
233+
},
234+
}
235+
`);
236+
});
237+
});

frontend/src/plugins/impl/anywidget/__tests__/model.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
vi,
1010
} from "vitest";
1111
import { TestUtils } from "@/__tests__/test-helpers";
12-
import type { Base64String } from "@/utils/json/base64";
1312
import {
1413
type AnyWidgetMessage,
1514
handleWidgetMessage,
@@ -246,7 +245,7 @@ describe("Model", () => {
246245
content,
247246
});
248247

249-
expect(callback).toHaveBeenCalledWith(content, undefined);
248+
expect(callback).toHaveBeenCalledWith(content, []);
250249
});
251250

252251
it("should handle custom messages with buffers", () => {
@@ -286,7 +285,7 @@ describe("ModelManager", () => {
286285
}: {
287286
modelId: string;
288287
message: AnyWidgetMessage;
289-
buffers: Base64String[];
288+
buffers: readonly DataView[];
290289
}) => {
291290
return handleWidgetMessage({
292291
modelId,
@@ -353,7 +352,7 @@ describe("ModelManager", () => {
353352
message: { method: "custom", content: { count: 1 } },
354353
buffers: [],
355354
});
356-
expect(callback).toHaveBeenCalledWith({ count: 1 }, undefined);
355+
expect(callback).toHaveBeenCalledWith({ count: 1 }, []);
357356
});
358357

359358
it("should handle close messages", async () => {

frontend/src/plugins/impl/anywidget/model.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { assertNever } from "@/utils/assertNever";
1010
import { Deferred } from "@/utils/Deferred";
1111
import { updateBufferPaths } from "@/utils/data-views";
1212
import { throwNotImplemented } from "@/utils/functions";
13-
import type { Base64String } from "@/utils/json/base64";
1413
import { Logger } from "@/utils/Logger";
1514

1615
export type EventHandler = (...args: any[]) => void;
@@ -171,7 +170,7 @@ export class Model<T extends Record<string, any>> implements AnyModel<T> {
171170
* When receiving a message from the backend.
172171
* We want to notify all listeners with `msg:custom`
173172
*/
174-
receiveCustomMessage(message: any, buffers?: DataView[]): void {
173+
receiveCustomMessage(message: any, buffers: readonly DataView[] = []): void {
175174
const response = AnyWidgetMessageSchema.safeParse(message);
176175
if (response.success) {
177176
const data = response.data;
@@ -260,7 +259,7 @@ export async function handleWidgetMessage({
260259
}: {
261260
modelId: string;
262261
msg: AnyWidgetMessage;
263-
buffers: Base64String[];
262+
buffers: readonly DataView[];
264263
modelManager: ModelManager;
265264
}): Promise<void> {
266265
if (msg.method === "echo_update") {

0 commit comments

Comments
 (0)