Conversation
📝 WalkthroughWalkthroughThe PR introduces comprehensive crash and error reporting across BlinkCard and BlinkID SDKs. Key additions include a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/blinkid-worker/src/BlinkIdWorker.ts (1)
415-499:⚠️ Potential issue | 🟠 MajorEarly init failures don't hit the same flush path.
After Line 416,
LicenseError, proxy-permission validation failures, andServerPermissionErrorcan all exit before the only failure flush at Line 498. That meansping.sdk.init.startis guaranteed to flush only forinitializeSdk()failures; the other init errors depend on a laterterminate()to ever be sent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid-worker/src/BlinkIdWorker.ts` around lines 415 - 499, The initial pinglet is being reported (reportPinglet) but only flushed in the catch for initializeSdk, so early throws (LicenseError, validateLicenseProxyPermissions, ServerPermissionError from obtainNewServerPermission/submitServerPermission) never call sendPinglets; wrap the entire init sequence (from the reportPinglet call through license checks, proxy validation/sanitization, server permission obtaining/submitServerPermission, and initializeSdk) in a single try/catch (or add small try/catches around each early-throwing block) that calls sendPinglets before rethrowing any caught error; reference functions/idents to update: reportPinglet, sendPinglets, LicenseError, validateLicenseProxyPermissions, sanitizeProxyUrls, obtainNewServerPermission, submitServerPermission (or `#wasmModule.submitServerPermission`), and initializeSdk.packages/blinkcard-ux-manager/src/core/test-helpers.integration.ts (1)
45-51:⚠️ Potential issue | 🟡 MinorResolve the session config once before building both collaborators.
When
options.sessionSettingsis omitted,createSessionSettings()is called separately for the fake session and forBlinkCardUxManager, so the helper can return two different settings objects. On top of that,sessionOverridesis broad enough to override config fields such assettingsor overlay flags on the fake session after the manager has already been initialized from separate values. That makes it easy to build an inconsistent test fixture.💡 Suggested fix
export type CreateBlinkCardIntegrationContextOptions = { sessionSettings?: BlinkCardSessionSettings; showDemoOverlay?: boolean; showProductionOverlay?: boolean; deviceInfo?: DeviceInfo; fakeCameraOptions?: CreateFakeCameraManagerOptions; - sessionOverrides?: Partial<BlinkCardSessionMock>; + sessionOverrides?: Omit< + Partial<BlinkCardSessionMock>, + "settings" | "showDemoOverlay" | "showProductionOverlay" + >; }; export const createBlinkCardIntegrationContext = ( options: CreateBlinkCardIntegrationContextOptions = {}, ): { manager: BlinkCardUxManager; fakeCameraManager: BlinkCardCameraHarness["fakeCameraManager"]; scanningSession: BlinkCardSessionMock; } => { const cameraHarness = createBlinkCardCameraHarness(options.fakeCameraOptions); + const sessionSettings = options.sessionSettings ?? createSessionSettings(); + const showDemoOverlay = options.showDemoOverlay ?? false; + const showProductionOverlay = options.showProductionOverlay ?? false; + const scanningSession = createFakeScanningSession< ProcessResultWithBuffer, BlinkCardSessionSettings, BlinkCardScanningResult >({ - settings: options.sessionSettings ?? createSessionSettings(), - showDemoOverlay: options.showDemoOverlay ?? false, - showProductionOverlay: options.showProductionOverlay ?? false, + settings: sessionSettings, + showDemoOverlay, + showProductionOverlay, overrides: options.sessionOverrides, }); const manager = new BlinkCardUxManager( cameraHarness.cameraManager, scanningSession as unknown as ConstructorParameters< typeof BlinkCardUxManager >[1], {}, - options.sessionSettings ?? createSessionSettings(), - options.showDemoOverlay ?? false, - options.showProductionOverlay ?? false, + sessionSettings, + showDemoOverlay, + showProductionOverlay, options.deviceInfo ?? createDeviceInfo(), );Also applies to: 75-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-ux-manager/src/core/test-helpers.integration.ts` around lines 45 - 51, Resolve the session settings once up-front and reuse them for both the fake session and the BlinkCardUxManager: compute let resolvedSessionSettings = options.sessionSettings ?? createSessionSettings(); then merge in any relevant sessionOverrides (e.g., overlay flags or settings fields from sessionOverrides) into that single object before constructing the fake BlinkCardSessionMock and before initializing BlinkCardUxManager so both collaborators receive the same settings; update code paths that currently call createSessionSettings() twice to use resolvedSessionSettings instead.
🧹 Nitpick comments (14)
packages/blinkcard-wasm/CHANGELOG.md (1)
3-4: Optional: add a brief entry under3000.0.3.A short bullet (e.g., dependency/version bump) improves release-note clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-wasm/CHANGELOG.md` around lines 3 - 4, Add a brief bullet under the existing "3000.0.3" heading in CHANGELOG.md summarizing the change for that release (e.g., "Dependency/version bump: upgraded X to Y" or a short note about what changed) so the release notes are clear and informative; update the section directly beneath the "## 3000.0.3" heading and keep the entry one concise sentence or bullet.packages/blinkid-wasm/CHANGELOG.md (1)
3-4: Consider adding at least one patch note under7.7.2.Empty release sections reduce traceability for consumers scanning changelogs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid-wasm/CHANGELOG.md` around lines 3 - 4, The release section "## 7.7.2" is empty; add at least one patch note immediately below that heading describing the change(s) in this patch (use the repo's existing changelog format—e.g., a short bullet or sentence with type/scope and description), referencing the "## 7.7.2" heading so consumers can see what changed in this release.packages/analytics/src/AnalyticService.test.ts (1)
56-97: LGTM!Good test coverage for the primary
logErrorEventpaths:
- Error instances with stack trace extraction
- Plain objects with JSON serialization
Consider adding edge case tests for completeness:
- Primitive error values (e.g.,
error: "string error")- Objects with circular references (to verify the
JSON.stringifyfallback)📋 Optional additional test cases
it("handles primitive error values", async () => { await analyticsService.logErrorEvent({ origin: "test", error: "simple string error", errorType: "NonFatal", }); expect(mockPingFn).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ errorMessage: "test: simple string error", stackTrace: undefined, }), }), ); }); it("handles circular reference objects", async () => { const circular: Record<string, unknown> = { a: 1 }; circular.self = circular; await analyticsService.logErrorEvent({ origin: "test", error: circular, errorType: "NonFatal", }); // Falls back to String() which produces "[object Object]" expect(mockPingFn).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ errorMessage: "test: [object Object]", }), }), ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/analytics/src/AnalyticService.test.ts` around lines 56 - 97, Add tests for two edge cases to the existing describe("logErrorEvent") block: one where error is a primitive string and one where error is an object with a circular reference. For the primitive test, call analyticsService.logErrorEvent({ origin: "test", error: "simple string error", errorType: "NonFatal" }) and assert mockPingFn was called with data.errorMessage "test: simple string error" and stackTrace undefined. For the circular test, create an object with a self-reference, pass it as error to analyticsService.logErrorEvent({ origin: "test", error: circular, errorType: "NonFatal" }) and assert mockPingFn was called with data.errorMessage that falls back to String(circular) (e.g., "test: [object Object]"). Use the existing mockPingFn and analyticsService.logErrorEvent identifiers so the new tests follow the current test patterns.packages/test-utils/src/mocks/wasmModuleFactory.ts (1)
79-85: Snapshot the overrides instead of storing a live reference.
lastModuleOverridescurrently points at the same object the caller passed in, so any later mutation by the code under test changes whatgetLastModuleOverrides()returns. Cloning on write makes this helper reflect the call-site input consistently.Suggested tweak
export default function createMockModule( moduleOverrides?: Record<string, unknown>, ): Promise< WasmModule<any, any> > { - lastModuleOverrides = moduleOverrides; + lastModuleOverrides = moduleOverrides + ? { ...moduleOverrides } + : undefined; if (!wasmModuleMock) { throw new Error( "Mock WASM module not set. Call setWasmModuleMock in test.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/test-utils/src/mocks/wasmModuleFactory.ts` around lines 79 - 85, The test util stores a live reference to the caller's moduleOverrides (assigned in createMockModule), so later mutations at the call site change getLastModuleOverrides() — change the assignment to store a clone instead of the original reference (e.g., shallow-copy the moduleOverrides when writing to lastModuleOverrides) so the helper consistently reflects the original input; update createMockModule (and any other place that writes lastModuleOverrides) to clone on write.packages/test-utils/src/vitest/cameraManager.ts (1)
37-47: Consider exposingemitErroron the harness API.The harness already wraps playback, frame, and state emitters. Adding the new error emitter here would keep tests from reaching through
fakeCameraManagerjust to drive the camera-error path.♻️ Suggested tweak
export type FakeCameraHarness<TCameraManager = FakeCameraManager> = { cameraManager: TCameraManager; fakeCameraManager: FakeCameraManager; emitPlaybackState: (playbackState: PlaybackState) => void; emitFrame: (imageData: ImageData) => Promise<ArrayBufferLike | void>; emitCameraState: (nextState: Partial<FakeCameraManagerState>) => void; + emitError: (error: Error) => void; setIsActive: (value: boolean) => void; stopFrameCapture: FakeCameraManager["stopFrameCapture"]; startFrameCapture: FakeCameraManager["startFrameCapture"]; startCameraStream: FakeCameraManager["startCameraStream"]; }; @@ return { cameraManager: fakeCameraManager as unknown as TCameraManager, fakeCameraManager, emitPlaybackState: (playbackState) => fakeCameraManager.emitPlaybackState(playbackState), emitFrame: (imageData) => fakeCameraManager.emitFrame(imageData), emitCameraState: (nextState) => fakeCameraManager.emitState(nextState), + emitError: (error) => fakeCameraManager.emitError(error), setIsActive: (value) => { fakeCameraManager.isActive = value; }, stopFrameCapture: fakeCameraManager.stopFrameCapture,Also applies to: 195-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/test-utils/src/vitest/cameraManager.ts` around lines 37 - 47, Add an emitError emitter to the FakeCameraHarness API so tests can trigger camera errors without poking into fakeCameraManager; update the FakeCameraHarness type to include emitError: (error: Error | string) => void and wire the harness implementation to delegate to the underlying FakeCameraManager.emitError method (or equivalent) wherever the harness object is constructed (also update the second occurrence noted around the other harness definition at lines ~195-214). Ensure the symbol names FakeCameraHarness and FakeCameraManager.emitError are used so callers can call harness.emitError(...) consistently.packages/blinkid/docs/classes/CameraManager.md (1)
260-278: Consider adding a one-line behavior description foraddErrorCallback().This section is missing the short explanatory text that neighboring methods include (e.g., what triggers the callback and what
booleanfrom cleanup indicates).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/docs/classes/CameraManager.md` around lines 260 - 278, Add a one-line behavior description to the CameraManager.md entry for addErrorCallback(): explain that addErrorCallback registers an ErrorCallback which is invoked when the camera or scanning subsystem encounters an error, and clarify that the returned cleanup function ((): boolean) removes the callback and returns true on successful removal (false otherwise); reference the addErrorCallback and ErrorCallback names and the boolean cleanup return in the single-line description.packages/blinkcard-worker/src/BlinkCardWorker.test.ts (1)
163-176: Consider asserting the “no ping reporting” behavior explicitly.
The test correctly checks rethrow semantics, but adding an explicit assertion that ping reporting is not invoked would lock in the intended no-wasm behavior and prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-worker/src/BlinkCardWorker.test.ts` around lines 163 - 176, The test should also assert that no ping reporting is invoked when wasm is not loaded: add a spy/mock for the worker's ping reporting method (e.g., worker.reportPing or the pingReporter used by BlinkCardWorker) before creating the proxy session, call proxySession.process(image) as already done, and then assert that the ping reporter mock was not called (expect(spy).not.toHaveBeenCalled()) to lock in the no-wasm behavior for BlinkCardWorker.createProxySession and proxySession.process.packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.ts (1)
75-100: Add tests for telemetry-failure branches too.This only locks down the “setup fails, telemetry succeeds” path. The new error-handling code also has dedicated behavior when
scanningSession.ping()orsendPinglets()reject, and those branches are where preserving the original setup error is easiest to break. A companion test for each rejection path would make this much harder to regress.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.ts` around lines 75 - 100, Add companion tests that simulate telemetry failures when creating the UX manager: one test should mock scanningSession.getSettings to reject with the original setup error (as in the existing test) but also mock scanningSession.ping to reject, and assert that createBlinkCardUxManager still rejects with the original setup error while ensuring a ping was attempted; another test should mock sendPinglets to reject and likewise assert the original setup error is preserved and that ping was called and sendPinglets was invoked. Use the same helpers and symbols (createBlinkCardUxManager, createFakeScanningSession, scanningSession.getSettings, scanningSession.ping, scanningSession.sendPinglets) to locate where to add these tests and mirror the existing expectation patterns for error content and call counts.packages/blinkid-ux-manager/src/core/BlinkIdUxManager.ts (1)
1031-1038: Consider makinghandleCameraManagerErrorprivate.This method is only used internally as a callback passed to
cameraManager.addErrorCallback(line 217). Making it private (#handleCameraManagerError) would be consistent with other internal handlers in this class (e.g.,#handleCameraPermissionChange,#handleHapticFeedback).♻️ Suggested refactor
- handleCameraManagerError = (error: Error) => { + `#handleCameraManagerError` = (error: Error) => { void this.#analytics.logErrorEvent({ origin: "cameraManager.error", error, errorType: "NonFatal", }); void this.#analytics.sendPinglets(); };And update line 217:
const removeCameraManagerErrorCallback = - this.cameraManager.addErrorCallback(this.handleCameraManagerError); + this.cameraManager.addErrorCallback(this.#handleCameraManagerError);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid-ux-manager/src/core/BlinkIdUxManager.ts` around lines 1031 - 1038, Rename the public method handleCameraManagerError to a private method `#handleCameraManagerError` in the BlinkIdUxManager class and update the cameraManager.addErrorCallback registration to pass this.#handleCameraManagerError; ensure the analytics calls inside the method remain unchanged and any other references to handleCameraManagerError are updated to the new private name so the callback continues to be called correctly.packages/blinkid/src/createBlinkIdUi.test.ts (1)
358-362: Consider simplifying redundant assertion.The
sessionNumberis already verified in theexpect.objectContainingmatcher at line 351. The additional extraction and assertion at lines 358-362 is redundant.🧹 Optional simplification
expect(mockReportPinglet).toHaveBeenCalledWith( expect.objectContaining({ schemaName: "ping.error", sessionNumber: 0, data: expect.objectContaining({ errorType: "Crash", errorMessage: "sdk.createBlinkId: ux failed", }), }), ); - const firstPinglet = mockReportPinglet.mock.calls[0]?.[0] as - | { sessionNumber?: number } - | undefined; - - expect(firstPinglet?.sessionNumber).toBe(0); expect(mockSendPinglets).toHaveBeenCalledTimes(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/src/createBlinkIdUi.test.ts` around lines 358 - 362, Remove the redundant assertion that extracts `firstPinglet` from `mockReportPinglet.mock.calls` and checks `firstPinglet?.sessionNumber` (the `firstPinglet` variable and the `expect(...sessionNumber).toBe(0)` assertion); the `sessionNumber` is already asserted via the existing `expect(...).toEqual(expect.objectContaining({ sessionNumber: 0 }))` (the `expect.objectContaining` matcher), so delete the extra extraction/assertion to simplify the test.packages/blinkcard/src/createBlinkCard.ts (1)
191-193: Remove unreachable code.Same issue as in
createBlinkIdUi.ts: this null check is unreachable. IfblinkCardCorewereundefined, theawait loadBlinkCardCore(...)at line 140 would have thrown, jumping to the catch block.🧹 Suggested fix
// selects the camera and starts the stream await cameraManager.startCameraStream(); - if (!blinkCardCore) { - throw new Error("BlinkCard core not initialized"); - } - - const loadedBlinkCardCore = blinkCardCore; + const loadedBlinkCardCore = blinkCardCore!;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard/src/createBlinkCard.ts` around lines 191 - 193, The null-check for blinkCardCore inside createBlinkCard (the if (!blinkCardCore) { throw new Error("BlinkCard core not initialized"); }) is unreachable because loadBlinkCardCore(...) already throws when core fails to load; remove this redundant guard from createBlinkCard.ts to avoid dead code and simplify control flow, leaving code flow to rely on loadBlinkCardCore and its error handling instead.packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.ts (1)
56-81: Consider always attempting to flush pinglets, even whenpingfails.Currently, if
scanningSession.ping(...)throws (lines 69-72), the code rethrows immediately without attemptingsendPinglets(). Other files in this PR (e.g.,createBlinkIdUi.ts,createBlinkCard.ts) attempt bothreportPingletandsendPingletsin a single try/catch, ensuring the flush is always attempted.If the intent is best-effort reporting, consider restructuring so
sendPinglets()is called regardless of whetherping()succeeds:♻️ Suggested refactor
} catch (error) { + let pingSucceeded = false; try { await scanningSession.ping({ schemaName: "ping.error", schemaVersion: "1.0.0", data: { errorType: "Crash", errorMessage: `ux.createBlinkCardUxManager: ${ error instanceof Error ? error.message : String(error) }`, stackTrace: error instanceof Error ? error.stack : undefined, }, }); + pingSucceeded = true; } catch (pingError) { console.warn("Failed to report error pinglet:", pingError); - throw error; } try { await scanningSession.sendPinglets(); } catch (sendError) { console.warn("Failed to flush error pinglets:", sendError); } throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.ts` around lines 56 - 81, The current catch block in createBlinkCardUxManager swallows the opportunity to flush pinglets when scanningSession.ping(...) fails; change the flow so scanningSession.sendPinglets() is always attempted (best-effort) even if scanningSession.ping throws: call scanningSession.ping inside its own try/catch to log/warn the ping error, then in a separate try (or finally) call scanningSession.sendPinglets() and log any send errors, and only after both attempts rethrow the original error; reference scanningSession.ping and scanningSession.sendPinglets in your changes.packages/blinkid/src/createBlinkIdUi.ts (1)
203-205: Remove unreachable code.This null check is unreachable. If
blinkIdCorewereundefined, theawait loadBlinkIdCore(...)at line 150 would have thrown, and execution would have jumped to the catch block. At line 203,blinkIdCoreis guaranteed to be defined.🧹 Suggested fix
// selects the camera and starts the stream await cameraManager.startCameraStream(); - if (!blinkIdCore) { - throw new Error("BlinkID core not initialized"); - } - - const loadedBlinkIdCore = blinkIdCore; + const loadedBlinkIdCore = blinkIdCore!;Or simply use
blinkIdCoredirectly since it's guaranteed to be defined at this point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/src/createBlinkIdUi.ts` around lines 203 - 205, Remove the redundant null-check for blinkIdCore in createBlinkIdUi: the await loadBlinkIdCore(...) call earlier (loadBlinkIdCore) will throw if it fails, so blinkIdCore is guaranteed to be defined by the time you reach the code that currently checks `if (!blinkIdCore) { throw new Error(...) }`; delete that unreachable check and use blinkIdCore directly in the subsequent logic (referencing the blinkIdCore variable inside the createBlinkIdUi flow).packages/blinkcard-worker/src/BlinkCardWorker.initBlinkCard.test.ts (1)
69-91: Consider extracting shared test helpers.The helper functions
getLatestWorkerListener,getLastQueuedPinglet, andgetLastQueuedPingletSessionNumberare duplicated between BlinkIdWorker and BlinkCardWorker test files. Consider extracting these to a shared test utility module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-worker/src/BlinkCardWorker.initBlinkCard.test.ts` around lines 69 - 91, Extract the duplicated test helpers into a shared test utilities module: create a new file exporting getLatestWorkerListener, getLastQueuedPinglet, and getLastQueuedPingletSessionNumber (preserve their signatures and typing such as ReturnType<typeof vi.fn>), replace the duplicated definitions in BlinkCardWorker.initBlinkCard.test.ts and BlinkIdWorker test with imports from that module, and update imports/usages so tests use the shared helpers instead of local copies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/blinkid-ux-manager/CHANGELOG.md`:
- Around line 41-51: Add blank lines immediately before and after the markdown
table so it is separated from the surrounding paragraph text to satisfy
markdownlint MD058; edit the CHANGELOG.md section that lists the renamed UI
state keys (references like SENSING_FRONT, SENSING_BACK, SCAN_BARCODE and their
new counterparts FRONT_PAGE_NOT_IN_FRAME, INTRO_FRONT_PAGE, PROCESSING_BARCODE,
etc.) and insert a single blank line above the table and another blank line
below it.
In `@packages/blinkid-ux-manager/src/core/createBlinkIdUxManager.ts`:
- Around line 56-77: The current code awaits scanningSession.ping(...) and
scanningSession.sendPinglets(), which can hang the init-failure path in
createBlinkIdUxManager; instead make telemetry best-effort and non-blocking by
launching a background helper (e.g., flushPingletsBestEffort or
reportCrashPinglet) that calls scanningSession.ping(...) and then
scanningSession.sendPinglets() but never blocks the original failure: it should
race each await with a short timeout, catch and log any errors, and return
without throwing; call that helper without awaiting it and then immediately
rethrow the original error so createBlinkIdUxManager fails fast.
In `@packages/blinkid-ux-manager/src/ui/BlinkIdFeedbackUi.tsx`:
- Around line 99-100: shouldShowFeedback currently returns only !isModalOpen(),
which ignores external pause/capture signals; update the visibility check to
require both no SDK modal and an explicit "feedback enabled" or "capture active"
flag (e.g. a new state/prop like feedbackEnabled or frameCaptureActive) so
feedback hides when host calls stopFrameCapture() or other pause paths. Locate
shouldShowFeedback and change its logic to combine !isModalOpen() with the new
boolean (and add wiring where capture start/stop occurs to set that flag, e.g.
in the code paths that call stopFrameCapture()/startFrameCapture()).
In `@packages/blinkid-worker/src/BlinkIdWorker.ts`:
- Around line 350-359: The catch block in BlinkIdWorker that calls
this.#wasmModule.queuePinglet currently logs the entire pinglet (including
sensitive userId) on failure; change the catch to avoid printing the full
pinglet and instead log only non-sensitive identifiers and error details (e.g.,
pinglet.schemaName, pinglet.schemaVersion, pinglet.sessionNumber or
this.#currentSessionNumber) plus the error message/stack as appropriate;
reference the queuePinglet call and the surrounding try/catch in BlinkIdWorker
and replace console.warn("Failed to queue pinglet:", error, pinglet) with a
sanitized log that omits or redacts pinglet.data and userId.
In `@packages/camera-manager/src/core/CameraManager.ts`:
- Around line 983-999: The runtime guard in CameraManager only accepts
ArrayBuffer but the FrameCaptureCallback type allows ArrayBufferLike, which
includes SharedArrayBuffer and will break at runtime; change the
FrameCaptureCallback type to explicitly return ArrayBuffer | void (or undefined)
instead of ArrayBufferLike, update its declaration(s) and any related type
aliases/usages (e.g. where FrameCaptureCallback is referenced), and adjust
callers if needed to return the underlying ArrayBuffer or nothing; keep the
runtime checks and reattach logic in `#videoFrameProcessor` unchanged.
In `@packages/unocss-config/package.json`:
- Line 3: The package.json in packages/unocss-config sets the "private" field as
a string ("true"); change this to a boolean true by replacing the quoted value
with an unquoted boolean so the "private" property is true (boolean) in
package.json.
In `@packages/worker-common/src/workerCrashReporter.ts`:
- Around line 44-62: The report function currently calls readSessionNumber() and
onError() directly inside the global error handler, which can throw and mask the
original crash; change report (the function named report) to guard both calls:
call readSessionNumber() inside a try/catch and fall back to 0 on error, then
invoke onError(...) inside its own try/catch so any exception is swallowed or
logged but not rethrown (still honor isReporting and ensure it is reset in
finally).
---
Outside diff comments:
In `@packages/blinkcard-ux-manager/src/core/test-helpers.integration.ts`:
- Around line 45-51: Resolve the session settings once up-front and reuse them
for both the fake session and the BlinkCardUxManager: compute let
resolvedSessionSettings = options.sessionSettings ?? createSessionSettings();
then merge in any relevant sessionOverrides (e.g., overlay flags or settings
fields from sessionOverrides) into that single object before constructing the
fake BlinkCardSessionMock and before initializing BlinkCardUxManager so both
collaborators receive the same settings; update code paths that currently call
createSessionSettings() twice to use resolvedSessionSettings instead.
In `@packages/blinkid-worker/src/BlinkIdWorker.ts`:
- Around line 415-499: The initial pinglet is being reported (reportPinglet) but
only flushed in the catch for initializeSdk, so early throws (LicenseError,
validateLicenseProxyPermissions, ServerPermissionError from
obtainNewServerPermission/submitServerPermission) never call sendPinglets; wrap
the entire init sequence (from the reportPinglet call through license checks,
proxy validation/sanitization, server permission
obtaining/submitServerPermission, and initializeSdk) in a single try/catch (or
add small try/catches around each early-throwing block) that calls sendPinglets
before rethrowing any caught error; reference functions/idents to update:
reportPinglet, sendPinglets, LicenseError, validateLicenseProxyPermissions,
sanitizeProxyUrls, obtainNewServerPermission, submitServerPermission (or
`#wasmModule.submitServerPermission`), and initializeSdk.
---
Nitpick comments:
In `@packages/analytics/src/AnalyticService.test.ts`:
- Around line 56-97: Add tests for two edge cases to the existing
describe("logErrorEvent") block: one where error is a primitive string and one
where error is an object with a circular reference. For the primitive test, call
analyticsService.logErrorEvent({ origin: "test", error: "simple string error",
errorType: "NonFatal" }) and assert mockPingFn was called with data.errorMessage
"test: simple string error" and stackTrace undefined. For the circular test,
create an object with a self-reference, pass it as error to
analyticsService.logErrorEvent({ origin: "test", error: circular, errorType:
"NonFatal" }) and assert mockPingFn was called with data.errorMessage that falls
back to String(circular) (e.g., "test: [object Object]"). Use the existing
mockPingFn and analyticsService.logErrorEvent identifiers so the new tests
follow the current test patterns.
In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.ts`:
- Around line 75-100: Add companion tests that simulate telemetry failures when
creating the UX manager: one test should mock scanningSession.getSettings to
reject with the original setup error (as in the existing test) but also mock
scanningSession.ping to reject, and assert that createBlinkCardUxManager still
rejects with the original setup error while ensuring a ping was attempted;
another test should mock sendPinglets to reject and likewise assert the original
setup error is preserved and that ping was called and sendPinglets was invoked.
Use the same helpers and symbols (createBlinkCardUxManager,
createFakeScanningSession, scanningSession.getSettings, scanningSession.ping,
scanningSession.sendPinglets) to locate where to add these tests and mirror the
existing expectation patterns for error content and call counts.
In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.ts`:
- Around line 56-81: The current catch block in createBlinkCardUxManager
swallows the opportunity to flush pinglets when scanningSession.ping(...) fails;
change the flow so scanningSession.sendPinglets() is always attempted
(best-effort) even if scanningSession.ping throws: call scanningSession.ping
inside its own try/catch to log/warn the ping error, then in a separate try (or
finally) call scanningSession.sendPinglets() and log any send errors, and only
after both attempts rethrow the original error; reference scanningSession.ping
and scanningSession.sendPinglets in your changes.
In `@packages/blinkcard-wasm/CHANGELOG.md`:
- Around line 3-4: Add a brief bullet under the existing "3000.0.3" heading in
CHANGELOG.md summarizing the change for that release (e.g., "Dependency/version
bump: upgraded X to Y" or a short note about what changed) so the release notes
are clear and informative; update the section directly beneath the "## 3000.0.3"
heading and keep the entry one concise sentence or bullet.
In `@packages/blinkcard-worker/src/BlinkCardWorker.initBlinkCard.test.ts`:
- Around line 69-91: Extract the duplicated test helpers into a shared test
utilities module: create a new file exporting getLatestWorkerListener,
getLastQueuedPinglet, and getLastQueuedPingletSessionNumber (preserve their
signatures and typing such as ReturnType<typeof vi.fn>), replace the duplicated
definitions in BlinkCardWorker.initBlinkCard.test.ts and BlinkIdWorker test with
imports from that module, and update imports/usages so tests use the shared
helpers instead of local copies.
In `@packages/blinkcard-worker/src/BlinkCardWorker.test.ts`:
- Around line 163-176: The test should also assert that no ping reporting is
invoked when wasm is not loaded: add a spy/mock for the worker's ping reporting
method (e.g., worker.reportPing or the pingReporter used by BlinkCardWorker)
before creating the proxy session, call proxySession.process(image) as already
done, and then assert that the ping reporter mock was not called
(expect(spy).not.toHaveBeenCalled()) to lock in the no-wasm behavior for
BlinkCardWorker.createProxySession and proxySession.process.
In `@packages/blinkcard/src/createBlinkCard.ts`:
- Around line 191-193: The null-check for blinkCardCore inside createBlinkCard
(the if (!blinkCardCore) { throw new Error("BlinkCard core not initialized"); })
is unreachable because loadBlinkCardCore(...) already throws when core fails to
load; remove this redundant guard from createBlinkCard.ts to avoid dead code and
simplify control flow, leaving code flow to rely on loadBlinkCardCore and its
error handling instead.
In `@packages/blinkid-ux-manager/src/core/BlinkIdUxManager.ts`:
- Around line 1031-1038: Rename the public method handleCameraManagerError to a
private method `#handleCameraManagerError` in the BlinkIdUxManager class and
update the cameraManager.addErrorCallback registration to pass
this.#handleCameraManagerError; ensure the analytics calls inside the method
remain unchanged and any other references to handleCameraManagerError are
updated to the new private name so the callback continues to be called
correctly.
In `@packages/blinkid-wasm/CHANGELOG.md`:
- Around line 3-4: The release section "## 7.7.2" is empty; add at least one
patch note immediately below that heading describing the change(s) in this patch
(use the repo's existing changelog format—e.g., a short bullet or sentence with
type/scope and description), referencing the "## 7.7.2" heading so consumers can
see what changed in this release.
In `@packages/blinkid/docs/classes/CameraManager.md`:
- Around line 260-278: Add a one-line behavior description to the
CameraManager.md entry for addErrorCallback(): explain that addErrorCallback
registers an ErrorCallback which is invoked when the camera or scanning
subsystem encounters an error, and clarify that the returned cleanup function
((): boolean) removes the callback and returns true on successful removal (false
otherwise); reference the addErrorCallback and ErrorCallback names and the
boolean cleanup return in the single-line description.
In `@packages/blinkid/src/createBlinkIdUi.test.ts`:
- Around line 358-362: Remove the redundant assertion that extracts
`firstPinglet` from `mockReportPinglet.mock.calls` and checks
`firstPinglet?.sessionNumber` (the `firstPinglet` variable and the
`expect(...sessionNumber).toBe(0)` assertion); the `sessionNumber` is already
asserted via the existing `expect(...).toEqual(expect.objectContaining({
sessionNumber: 0 }))` (the `expect.objectContaining` matcher), so delete the
extra extraction/assertion to simplify the test.
In `@packages/blinkid/src/createBlinkIdUi.ts`:
- Around line 203-205: Remove the redundant null-check for blinkIdCore in
createBlinkIdUi: the await loadBlinkIdCore(...) call earlier (loadBlinkIdCore)
will throw if it fails, so blinkIdCore is guaranteed to be defined by the time
you reach the code that currently checks `if (!blinkIdCore) { throw new
Error(...) }`; delete that unreachable check and use blinkIdCore directly in the
subsequent logic (referencing the blinkIdCore variable inside the
createBlinkIdUi flow).
In `@packages/test-utils/src/mocks/wasmModuleFactory.ts`:
- Around line 79-85: The test util stores a live reference to the caller's
moduleOverrides (assigned in createMockModule), so later mutations at the call
site change getLastModuleOverrides() — change the assignment to store a clone
instead of the original reference (e.g., shallow-copy the moduleOverrides when
writing to lastModuleOverrides) so the helper consistently reflects the original
input; update createMockModule (and any other place that writes
lastModuleOverrides) to clone on write.
In `@packages/test-utils/src/vitest/cameraManager.ts`:
- Around line 37-47: Add an emitError emitter to the FakeCameraHarness API so
tests can trigger camera errors without poking into fakeCameraManager; update
the FakeCameraHarness type to include emitError: (error: Error | string) => void
and wire the harness implementation to delegate to the underlying
FakeCameraManager.emitError method (or equivalent) wherever the harness object
is constructed (also update the second occurrence noted around the other harness
definition at lines ~195-214). Ensure the symbol names FakeCameraHarness and
FakeCameraManager.emitError are used so callers can call harness.emitError(...)
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a35221de-e015-43ec-9eb7-eab54bd1ae05
⛔ Files ignored due to path filters (16)
packages/blinkcard-wasm/dist/advanced-threads/BlinkCardModule.jsis excluded by!**/dist/**packages/blinkcard-wasm/dist/advanced-threads/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/advanced/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/basic/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/size-manifest.jsonis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced-threads/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced-threads/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/full/advanced/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/full/basic/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/basic/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/advanced-threads/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/lightweight/advanced-threads/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/advanced/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/basic/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/size-manifest.jsonis excluded by!**/dist/**
📒 Files selected for processing (84)
.gitignorepackages/analytics/CHANGELOG.mdpackages/analytics/package.jsonpackages/analytics/src/AnalyticService.test.tspackages/analytics/src/AnalyticService.tspackages/blinkcard-core/CHANGELOG.mdpackages/blinkcard-core/docs/classes/BlinkCardWorker.mdpackages/blinkcard-core/package.jsonpackages/blinkcard-ux-manager/CHANGELOG.mdpackages/blinkcard-ux-manager/package.jsonpackages/blinkcard-ux-manager/src/core/BlinkCardUxManager.test.tspackages/blinkcard-ux-manager/src/core/BlinkCardUxManager.tspackages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.tspackages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.tspackages/blinkcard-ux-manager/src/core/test-helpers.integration.tspackages/blinkcard-ux-manager/src/ui/BlinkCardFeedbackUi.tsxpackages/blinkcard-wasm/CHANGELOG.mdpackages/blinkcard-wasm/package.jsonpackages/blinkcard-worker/CHANGELOG.mdpackages/blinkcard-worker/docs/classes/BlinkCardWorker.mdpackages/blinkcard-worker/package.jsonpackages/blinkcard-worker/src/BlinkCardWorker.initBlinkCard.test.tspackages/blinkcard-worker/src/BlinkCardWorker.test.tspackages/blinkcard-worker/src/BlinkCardWorker.tspackages/blinkcard/CHANGELOG.mdpackages/blinkcard/docs/README.mdpackages/blinkcard/docs/classes/BlinkCardWorker.mdpackages/blinkcard/docs/classes/CameraManager.mdpackages/blinkcard/docs/type-aliases/ErrorCallback.mdpackages/blinkcard/package.jsonpackages/blinkcard/src/createBlinkCard.test.tspackages/blinkcard/src/createBlinkCard.tspackages/blinkid-core/CHANGELOG.mdpackages/blinkid-core/docs/classes/BlinkIdWorker.mdpackages/blinkid-core/package.jsonpackages/blinkid-ux-manager/CHANGELOG.mdpackages/blinkid-ux-manager/docs/interfaces/BlinkIdUxManager.mdpackages/blinkid-ux-manager/package.jsonpackages/blinkid-ux-manager/src/core/BlinkIdUxManager.test.tspackages/blinkid-ux-manager/src/core/BlinkIdUxManager.tspackages/blinkid-ux-manager/src/core/createBlinkIdUxManager.test.tspackages/blinkid-ux-manager/src/core/createBlinkIdUxManager.tspackages/blinkid-ux-manager/src/core/test-helpers.integration.tspackages/blinkid-ux-manager/src/ui/BlinkIdFeedbackUi.tsxpackages/blinkid-wasm/CHANGELOG.mdpackages/blinkid-wasm/package.jsonpackages/blinkid-worker/CHANGELOG.mdpackages/blinkid-worker/docs/classes/BlinkIdWorker.mdpackages/blinkid-worker/package.jsonpackages/blinkid-worker/src/BlinkIdWorker.initBlinkId.test.tspackages/blinkid-worker/src/BlinkIdWorker.test.tspackages/blinkid-worker/src/BlinkIdWorker.tspackages/blinkid/CHANGELOG.mdpackages/blinkid/docs/README.mdpackages/blinkid/docs/classes/BlinkIdWorker.mdpackages/blinkid/docs/classes/CameraManager.mdpackages/blinkid/docs/interfaces/BlinkIdUxManager.mdpackages/blinkid/docs/type-aliases/ErrorCallback.mdpackages/blinkid/package.jsonpackages/blinkid/src/createBlinkIdUi.test.tspackages/blinkid/src/createBlinkIdUi.tspackages/camera-manager/CHANGELOG.mdpackages/camera-manager/docs/README.mdpackages/camera-manager/docs/classes/CameraManager.mdpackages/camera-manager/docs/type-aliases/ErrorCallback.mdpackages/camera-manager/package.jsonpackages/camera-manager/src/core/CameraManager.tspackages/core-common/CHANGELOG.mdpackages/core-common/package.jsonpackages/core-common/src/createProxyWorker.tspackages/feedback-stabilizer/package.jsonpackages/shared-components/package.jsonpackages/test-utils/package.jsonpackages/test-utils/src/mocks/wasmModuleFactory.tspackages/test-utils/src/vitest/cameraManager.test.tspackages/test-utils/src/vitest/cameraManager.tspackages/tsconfig/package.jsonpackages/unocss-config/package.jsonpackages/ux-common/package.jsonpackages/wasm-common/package.jsonpackages/worker-common/CHANGELOG.mdpackages/worker-common/package.jsonpackages/worker-common/src/workerCrashReporter.test.tspackages/worker-common/src/workerCrashReporter.ts
| - Renames several UI state keys. Integrations that reference state keys by name should update accordingly. Each `SENSING_*` state has been split into a framing-feedback state (`*_NOT_IN_FRAME`) and a new intro guidance state (`INTRO_*`): | ||
| | Old key | New key(s) | | ||
| |---|---| | ||
| | `SENSING_FRONT` | `FRONT_PAGE_NOT_IN_FRAME`, `INTRO_FRONT_PAGE` | | ||
| | `SENSING_BACK` | `BACK_PAGE_NOT_IN_FRAME`, `INTRO_BACK_PAGE` | | ||
| | `SENSING_DATA_PAGE` | `DATA_PAGE_NOT_IN_FRAME`, `INTRO_DATA_PAGE` | | ||
| | `SENSING_TOP_PAGE` | `TOP_PAGE_NOT_IN_FRAME`, `INTRO_TOP_PAGE` | | ||
| | `SENSING_LEFT_PAGE` | `LEFT_PAGE_NOT_IN_FRAME`, `INTRO_LEFT_PAGE` | | ||
| | `SENSING_RIGHT_PAGE` | `RIGHT_PAGE_NOT_IN_FRAME`, `INTRO_RIGHT_PAGE` | | ||
| | `SENSING_LAST_PAGE` | `LAST_PAGE_NOT_IN_FRAME`, `INTRO_LAST_PAGE` | | ||
| | `SCAN_BARCODE` | `PROCESSING_BARCODE` | |
There was a problem hiding this comment.
Add blank lines around the table to comply with markdown linting.
Static analysis (markdownlint MD058) flags that tables should be surrounded by blank lines for proper rendering in all markdown parsers.
📝 Suggested fix
- Renames several UI state keys. Integrations that reference state keys by name should update accordingly. Each `SENSING_*` state has been split into a framing-feedback state (`*_NOT_IN_FRAME`) and a new intro guidance state (`INTRO_*`):
+
| Old key | New key(s) |
|---|---|
| `SENSING_FRONT` | `FRONT_PAGE_NOT_IN_FRAME`, `INTRO_FRONT_PAGE` |
| `SENSING_BACK` | `BACK_PAGE_NOT_IN_FRAME`, `INTRO_BACK_PAGE` |
| `SENSING_DATA_PAGE` | `DATA_PAGE_NOT_IN_FRAME`, `INTRO_DATA_PAGE` |
| `SENSING_TOP_PAGE` | `TOP_PAGE_NOT_IN_FRAME`, `INTRO_TOP_PAGE` |
| `SENSING_LEFT_PAGE` | `LEFT_PAGE_NOT_IN_FRAME`, `INTRO_LEFT_PAGE` |
| `SENSING_RIGHT_PAGE` | `RIGHT_PAGE_NOT_IN_FRAME`, `INTRO_RIGHT_PAGE` |
| `SENSING_LAST_PAGE` | `LAST_PAGE_NOT_IN_FRAME`, `INTRO_LAST_PAGE` |
| `SCAN_BARCODE` | `PROCESSING_BARCODE` |
+
- Updated dependencies📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Renames several UI state keys. Integrations that reference state keys by name should update accordingly. Each `SENSING_*` state has been split into a framing-feedback state (`*_NOT_IN_FRAME`) and a new intro guidance state (`INTRO_*`): | |
| | Old key | New key(s) | | |
| |---|---| | |
| | `SENSING_FRONT` | `FRONT_PAGE_NOT_IN_FRAME`, `INTRO_FRONT_PAGE` | | |
| | `SENSING_BACK` | `BACK_PAGE_NOT_IN_FRAME`, `INTRO_BACK_PAGE` | | |
| | `SENSING_DATA_PAGE` | `DATA_PAGE_NOT_IN_FRAME`, `INTRO_DATA_PAGE` | | |
| | `SENSING_TOP_PAGE` | `TOP_PAGE_NOT_IN_FRAME`, `INTRO_TOP_PAGE` | | |
| | `SENSING_LEFT_PAGE` | `LEFT_PAGE_NOT_IN_FRAME`, `INTRO_LEFT_PAGE` | | |
| | `SENSING_RIGHT_PAGE` | `RIGHT_PAGE_NOT_IN_FRAME`, `INTRO_RIGHT_PAGE` | | |
| | `SENSING_LAST_PAGE` | `LAST_PAGE_NOT_IN_FRAME`, `INTRO_LAST_PAGE` | | |
| | `SCAN_BARCODE` | `PROCESSING_BARCODE` | | |
| - Renames several UI state keys. Integrations that reference state keys by name should update accordingly. Each `SENSING_*` state has been split into a framing-feedback state (`*_NOT_IN_FRAME`) and a new intro guidance state (`INTRO_*`): | |
| | Old key | New key(s) | | |
| |---|---| | |
| | `SENSING_FRONT` | `FRONT_PAGE_NOT_IN_FRAME`, `INTRO_FRONT_PAGE` | | |
| | `SENSING_BACK` | `BACK_PAGE_NOT_IN_FRAME`, `INTRO_BACK_PAGE` | | |
| | `SENSING_DATA_PAGE` | `DATA_PAGE_NOT_IN_FRAME`, `INTRO_DATA_PAGE` | | |
| | `SENSING_TOP_PAGE` | `TOP_PAGE_NOT_IN_FRAME`, `INTRO_TOP_PAGE` | | |
| | `SENSING_LEFT_PAGE` | `LEFT_PAGE_NOT_IN_FRAME`, `INTRO_LEFT_PAGE` | | |
| | `SENSING_RIGHT_PAGE` | `RIGHT_PAGE_NOT_IN_FRAME`, `INTRO_RIGHT_PAGE` | | |
| | `SENSING_LAST_PAGE` | `LAST_PAGE_NOT_IN_FRAME`, `INTRO_LAST_PAGE` | | |
| | `SCAN_BARCODE` | `PROCESSING_BARCODE` | | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 42-42: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 51-51: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkid-ux-manager/CHANGELOG.md` around lines 41 - 51, Add blank
lines immediately before and after the markdown table so it is separated from
the surrounding paragraph text to satisfy markdownlint MD058; edit the
CHANGELOG.md section that lists the renamed UI state keys (references like
SENSING_FRONT, SENSING_BACK, SCAN_BARCODE and their new counterparts
FRONT_PAGE_NOT_IN_FRAME, INTRO_FRONT_PAGE, PROCESSING_BARCODE, etc.) and insert
a single blank line above the table and another blank line below it.
| // TODO: Cover cases where frame processing is paused by 3rd party modal dialogs | ||
| const shouldShowFeedback = () => !isModalOpen(); |
There was a problem hiding this comment.
Keep an explicit pause/capture signal in the feedback visibility check.
shouldShowFeedback() now depends only on !isModalOpen(). Any host-driven pause path outside these SDK modal flags—e.g. a parent dialog or manual stopFrameCapture()—will keep showing live guidance even though scanning is stalled. Consider combining modal visibility with an explicit “feedback enabled” state instead of dropping capture-state awareness completely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkid-ux-manager/src/ui/BlinkIdFeedbackUi.tsx` around lines 99 -
100, shouldShowFeedback currently returns only !isModalOpen(), which ignores
external pause/capture signals; update the visibility check to require both no
SDK modal and an explicit "feedback enabled" or "capture active" flag (e.g. a
new state/prop like feedbackEnabled or frameCaptureActive) so feedback hides
when host calls stopFrameCapture() or other pause paths. Locate
shouldShowFeedback and change its logic to combine !isModalOpen() with the new
boolean (and add wiring where capture start/stop occurs to set that flag, e.g.
in the code paths that call stopFrameCapture()/startFrameCapture()).
| try { | ||
| this.#wasmModule.queuePinglet( | ||
| JSON.stringify(data), | ||
| schemaName, | ||
| schemaVersion, | ||
| sessionNumber!, // we know sesion number is provided because we're using proxy function | ||
| JSON.stringify(pinglet.data), | ||
| pinglet.schemaName, | ||
| pinglet.schemaVersion, | ||
| pinglet.sessionNumber ?? this.#currentSessionNumber, | ||
| ); | ||
| } catch (error) { | ||
| console.warn("Failed to queue pinglet:", error, { | ||
| data, | ||
| schemaName, | ||
| schemaVersion, | ||
| sessionNumber, | ||
| }); | ||
| console.warn("Failed to queue pinglet:", error, pinglet); | ||
| } |
There was a problem hiding this comment.
Don't log the full pinglet on queue failures.
Line 358 logs the raw pinglet, and ping.sdk.init.start includes userId. This leaks analytics payloads, and potentially stack traces, into the browser console whenever queuePinglet() throws.
🛡️ Safer logging
} catch (error) {
- console.warn("Failed to queue pinglet:", error, pinglet);
+ console.warn("Failed to queue pinglet:", error, {
+ schemaName: pinglet.schemaName,
+ schemaVersion: pinglet.schemaVersion,
+ sessionNumber: pinglet.sessionNumber ?? this.#currentSessionNumber,
+ });
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| this.#wasmModule.queuePinglet( | |
| JSON.stringify(data), | |
| schemaName, | |
| schemaVersion, | |
| sessionNumber!, // we know sesion number is provided because we're using proxy function | |
| JSON.stringify(pinglet.data), | |
| pinglet.schemaName, | |
| pinglet.schemaVersion, | |
| pinglet.sessionNumber ?? this.#currentSessionNumber, | |
| ); | |
| } catch (error) { | |
| console.warn("Failed to queue pinglet:", error, { | |
| data, | |
| schemaName, | |
| schemaVersion, | |
| sessionNumber, | |
| }); | |
| console.warn("Failed to queue pinglet:", error, pinglet); | |
| } | |
| try { | |
| this.#wasmModule.queuePinglet( | |
| JSON.stringify(pinglet.data), | |
| pinglet.schemaName, | |
| pinglet.schemaVersion, | |
| pinglet.sessionNumber ?? this.#currentSessionNumber, | |
| ); | |
| } catch (error) { | |
| console.warn("Failed to queue pinglet:", error, { | |
| schemaName: pinglet.schemaName, | |
| schemaVersion: pinglet.schemaVersion, | |
| sessionNumber: pinglet.sessionNumber ?? this.#currentSessionNumber, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkid-worker/src/BlinkIdWorker.ts` around lines 350 - 359, The
catch block in BlinkIdWorker that calls this.#wasmModule.queuePinglet currently
logs the entire pinglet (including sensitive userId) on failure; change the
catch to avoid printing the full pinglet and instead log only non-sensitive
identifiers and error details (e.g., pinglet.schemaName, pinglet.schemaVersion,
pinglet.sessionNumber or this.#currentSessionNumber) plus the error
message/stack as appropriate; reference the queuePinglet call and the
surrounding try/catch in BlinkIdWorker and replace console.warn("Failed to queue
pinglet:", error, pinglet) with a sanitized log that omits or redacts
pinglet.data and userId.
| if (!(returnedBuffer instanceof ArrayBuffer)) { | ||
| if (ArrayBuffer.isView(returnedBuffer)) { | ||
| this.#videoFrameProcessor.reattachArrayBuffer( | ||
| returnedBuffer.buffer, | ||
| ); | ||
| } | ||
|
|
||
| throw new Error( | ||
| stripIndents` | ||
| Frame capture callback did not return an ArrayBuffer. | ||
| Make sure to return the underlying buffer, not the view. | ||
| `, | ||
| ); | ||
| } | ||
|
|
||
| // Return the buffer to the processor | ||
| this.#videoFrameProcessor.reattachArrayBuffer(returnedBuffer); | ||
| // Return the buffer to the processor | ||
| this.#videoFrameProcessor.reattachArrayBuffer(returnedBuffer); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the public type/runtime mismatch for frame capture callback return values.
rg -n -C2 'FrameCaptureCallback|ArrayBufferLike|instanceof ArrayBuffer' packages/camera-manager/src/core/CameraManager.tsRepository: microblink/web-sdks
Length of output: 929
Narrow the callback contract to match runtime validation.
FrameCaptureCallback is typed to return ArrayBufferLike, but the runtime guard at line 983 only accepts ArrayBuffer. Since ArrayBufferLike includes SharedArrayBuffer, a callback returning SharedArrayBuffer will pass type checking and fail at runtime. Update the callback type to ArrayBuffer | void to match the actual runtime constraint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/camera-manager/src/core/CameraManager.ts` around lines 983 - 999,
The runtime guard in CameraManager only accepts ArrayBuffer but the
FrameCaptureCallback type allows ArrayBufferLike, which includes
SharedArrayBuffer and will break at runtime; change the FrameCaptureCallback
type to explicitly return ArrayBuffer | void (or undefined) instead of
ArrayBufferLike, update its declaration(s) and any related type aliases/usages
(e.g. where FrameCaptureCallback is referenced), and adjust callers if needed to
return the underlying ArrayBuffer or nothing; keep the runtime checks and
reattach logic in `#videoFrameProcessor` unchanged.
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "@microblink/unocss-config", | |||
| "private": "true", | |||
There was a problem hiding this comment.
Use boolean instead of string for the private field.
The private field should be a boolean true rather than the string "true". While npm may tolerate the string value, the package.json schema specifies this should be a boolean.
📋 Proposed fix
- "private": "true",
+ "private": true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "private": "true", | |
| "private": true, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/unocss-config/package.json` at line 3, The package.json in
packages/unocss-config sets the "private" field as a string ("true"); change
this to a boolean true by replacing the quoted value with an unquoted boolean so
the "private" property is true (boolean) in package.json.
50612bc to
1ca90dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/blinkcard-worker/src/BlinkCardWorker.ts (1)
318-332:⚠️ Potential issue | 🟠 MajorSanitize the queue-failure log payload.
Logging the whole
pinglethere leaksuserIdfromping.sdk.init.startand stack traces fromping.errorwheneverqueuePinglet()fails. Log schema/session metadata only.Proposed fix
} catch (error) { - console.warn("Failed to queue pinglet:", error, pinglet); + console.warn("Failed to queue pinglet:", error, { + schemaName: pinglet.schemaName, + schemaVersion: pinglet.schemaVersion, + sessionNumber: pinglet.sessionNumber ?? this.#currentSessionNumber, + }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard-worker/src/BlinkCardWorker.ts` around lines 318 - 332, In reportPinglet, stop logging the full pinglet on queue failure and instead log only schema/session metadata and a non-sensitive error string: in the catch block for this.#wasmModule.queuePinglet(...) (function reportPinglet / queuePinglet), replace console.warn("Failed to queue pinglet:", error, pinglet) with a call that builds a sanitized payload like { schemaName: pinglet.schemaName, schemaVersion: pinglet.schemaVersion, sessionNumber: pinglet.sessionNumber ?? this.#currentSessionNumber } and include only the error message (e.g. const errMsg = error instanceof Error ? error.message : String(error)); then console.warn with a short message and that sanitized object so no userId or stack traces are logged.
🧹 Nitpick comments (5)
packages/blinkcard/docs/classes/CameraManager.md (1)
260-278: Looks correct; consider adding a one-line behavior description for consistency.The new
addErrorCallback(errorCallback): () => booleandocs match the implementation contract and type usage. Optional polish: add a short sentence (like inaddFrameCaptureCallback) explaining when the callback is invoked and that the returned function unsubscribes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkcard/docs/classes/CameraManager.md` around lines 260 - 278, Add a one-line behavioral description to the addErrorCallback() doc entry (similar to addFrameCaptureCallback) that states when the provided errorCallback is invoked (on camera/error events) and that the function returned by addErrorCallback unsubscribes the callback; update the documentation text around the addErrorCallback signature to include that sentence so readers understand invocation timing and that the returned () => boolean removes the subscription.packages/blinkid/docs/classes/CameraManager.md (1)
260-278: Consider adding a description foraddErrorCallback.Other methods like
addFrameCaptureCallback(line 284) include descriptions explaining their purpose and behavior. For consistency and developer experience, consider adding a brief description explaining when the error callback is invoked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/docs/classes/CameraManager.md` around lines 260 - 278, The documentation for addErrorCallback lacks a descriptive summary; add a brief sentence or two above the parameters describing its purpose and invocation conditions (e.g., when camera or streaming errors occur), mirroring the style used by addFrameCaptureCallback; update the CameraManager.md addErrorCallback section to state when the ErrorCallback is called, what kinds of errors are passed, and any return semantics (the boolean returned by the callback) so consumers understand behavior of addErrorCallback and its returned () => boolean unsubscription function.packages/blinkid/src/createBlinkIdUi.test.ts (1)
339-366: Consider removing redundant sessionNumber assertion.Lines 358-362 extract and assert
firstPinglet?.sessionNumber === 0, but this is already verified by theexpect.objectContaining({ sessionNumber: 0 })matcher on line 349. The redundant assertion adds noise without additional value.♻️ Proposed simplification
expect(mockReportPinglet).toHaveBeenCalledWith( expect.objectContaining({ schemaName: "ping.error", sessionNumber: 0, data: expect.objectContaining({ errorType: "Crash", errorMessage: "sdk.createBlinkId: ux failed", }), }), ); - const firstPinglet = mockReportPinglet.mock.calls[0]?.[0] as - | { sessionNumber?: number } - | undefined; - - expect(firstPinglet?.sessionNumber).toBe(0); expect(mockSendPinglets).toHaveBeenCalledTimes(1); expect(scanningSession.ping).not.toHaveBeenCalled(); expect(scanningSession.sendPinglets).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/src/createBlinkIdUi.test.ts` around lines 339 - 366, The test "best-effort reports crashes through the core after session creation" contains a redundant assertion: remove the extraction of firstPinglet and the expect(firstPinglet?.sessionNumber).toBe(0) check because sessionNumber is already asserted via expect.objectContaining({ sessionNumber: 0 }) on the mockReportPinglet call; keep the rest of the test (mock setup, createBlinkId call, mockReportPinglet/sendPinglets/scanningSession assertions) and delete only the firstPinglet variable extraction and its expect assertion.packages/blinkid/src/createBlinkIdUi.ts (1)
203-205: Redundant null check -blinkIdCoreis guaranteed to be defined here.At this point in the code flow,
blinkIdCorewas assigned on line 150 and used on line 159. If either had failed, execution would have jumped to the catch block. This check is unreachable dead code.♻️ Proposed fix to remove dead code
// selects the camera and starts the stream await cameraManager.startCameraStream(); - if (!blinkIdCore) { - throw new Error("BlinkID core not initialized"); - } - const loadedBlinkIdCore = blinkIdCore; const destroy = async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid/src/createBlinkIdUi.ts` around lines 203 - 205, Remove the unreachable null-check for blinkIdCore in createBlinkIdUi (the if (!blinkIdCore) throw new Error(...) block) because blinkIdCore is assigned earlier and any failure would already exit to the catch; simply delete that conditional and its throw; if you want safety retain an assertion-style comment or use a TypeScript non-null assertion where blinkIdCore is referenced instead of keeping this dead runtime check.packages/blinkid-ux-manager/src/core/test-helpers.integration.ts (1)
83-86: Consider adding a comment explaining the type assertion.The double type assertion (
as unknown as ...) bypasses TypeScript's type checking. While acceptable for test utilities where mock types don't perfectly align with production types, a brief comment would help future maintainers understand why this is necessary and safe.📝 Suggested documentation
- const manager = await createBlinkIdUxManager( - cameraHarness.cameraManager, - scanningSession as unknown as Parameters<typeof createBlinkIdUxManager>[1], - ); + // Type assertion needed: FakeScanningSession implements the same interface + // as the real session but TypeScript can't infer this from the generic types + const manager = await createBlinkIdUxManager( + cameraHarness.cameraManager, + scanningSession as unknown as Parameters<typeof createBlinkIdUxManager>[1], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blinkid-ux-manager/src/core/test-helpers.integration.ts` around lines 83 - 86, Add a brief inline comment next to the double type assertion used when calling createBlinkIdUxManager with scanningSession (e.g., `scanningSession as unknown as Parameters<typeof createBlinkIdUxManager>[1]`) explaining that the double assertion is intentional to bypass strict TypeScript mismatches in test mocks, that the runtime shape is compatible for the test harness (cameraHarness.cameraManager), and that this is safe because it’s limited to integration test utilities; place the comment directly above or beside the assertion near the createBlinkIdUxManager call so future maintainers understand why the cast is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/blinkcard-ux-manager/src/core/BlinkCardUxManager.ts`:
- Around line 718-725: The catch blocks currently await
this.#analytics.logErrorEvent(...) and this.#analytics.sendPinglets(), which can
cause telemetry failures to replace the original error; change telemetry calls
in the catch blocks (where the caught variable is error) to be best-effort:
invoke logErrorEvent and sendPinglets without letting their failures overwrite
the original flow—either kick them off without awaiting or await them inside
their own try/catch and swallow/log telemetry errors locally—then immediately
rethrow the original error so process()/getResult() callers receive the original
exception; specifically update the catch blocks that call
this.#analytics.logErrorEvent and this.#analytics.sendPinglets (including the
other occurrence around lines 978-987) to implement this pattern.
- Around line 733-740: The camera-manager error handler starts sendPinglets()
before the async logErrorEvent() finishes, so the flush can occur before the
error ping is enqueued; change the private handler `#handleCameraManagerError` to
be async (or return the log promise) and await
this.#analytics.logErrorEvent(...) before calling
this.#analytics.sendPinglets(), ensuring the error ping is queued prior to
flushing (update any callers if they rely on synchronous behavior).
In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.ts`:
- Around line 56-81: The current catch block rethrows the original error
immediately when scanningSession.ping() fails, which prevents
scanningSession.sendPinglets() from being attempted; change the control flow so
sendPinglets() is always attempted regardless of whether ping() succeeded or
threw: capture the original error in the outer catch (the variable named error),
do NOT rethrow inside the inner catch (pingError), and move the throw of the
original error to after the sendPinglets() attempt (or use a finally that calls
scanningSession.sendPinglets() and then rethrows the original error). Update the
code around scanningSession.ping, the pingError catch, and the
scanningSession.sendPinglets() call to implement this guaranteed "best-effort"
flush behavior.
In `@packages/blinkcard-worker/src/BlinkCardWorker.ts`:
- Around line 702-717: In terminate(), do not early-return from the
session-delete catch when checking this.#wasmModule — remove the `return` (or
move the error-reporting logic out of the conditional) so that
reportPinglet/sendPinglets can run but the cleanup block (uninstalling crash
reporter and calling self.close()) always executes; alternatively wrap the
session deletion and error reporting in a try/catch/finally so that the final
cleanup (uninstall crash reporter, call self.close()) runs regardless of whether
`#wasmModule` is present or session deletion throws (affecting symbols:
terminate(), this.#wasmModule, reportPinglet, sendPinglets, crash reporter
uninstall, self.close()).
In `@packages/blinkcard/src/createBlinkCard.ts`:
- Around line 153-166: The try path can leave partially initialized resources
(core, cameraManager, cameraUi, blinkCardUxManager) undisposed if an error is
thrown after loadBlinkCardCore(); modify the init flow around
loadBlinkCardCore(), cameraManager, createBlinkCardUxManager, and
createCameraManagerUi so that any resources successfully constructed before the
error are properly cleaned up (call their destroy/cleanup methods) before
rethrowing; ensure you capture references to cameraManager, blinkCardUxManager,
cameraUi and the core client in outer scope so the catch/finally can call
destroy() on whichever were created and still propagate the original error.
In `@packages/blinkid-worker/src/BlinkIdWorker.ts`:
- Around line 818-833: In terminate() the catch block currently returns when
this.#wasmModule is falsy, which skips the shutdown path (removal of the crash
reporter and self.close()); instead remove the early return and let execution
continue to the shutdown logic so the crash reporter is removed and self.close()
runs; apply the same change to the second catch (lines 839-847) so that after
reporting the pinglet via reportPinglet(...) and sendPinglets() the function
does not return early even if this.#wasmModule is missing or delete() threw.
---
Duplicate comments:
In `@packages/blinkcard-worker/src/BlinkCardWorker.ts`:
- Around line 318-332: In reportPinglet, stop logging the full pinglet on queue
failure and instead log only schema/session metadata and a non-sensitive error
string: in the catch block for this.#wasmModule.queuePinglet(...) (function
reportPinglet / queuePinglet), replace console.warn("Failed to queue pinglet:",
error, pinglet) with a call that builds a sanitized payload like { schemaName:
pinglet.schemaName, schemaVersion: pinglet.schemaVersion, sessionNumber:
pinglet.sessionNumber ?? this.#currentSessionNumber } and include only the error
message (e.g. const errMsg = error instanceof Error ? error.message :
String(error)); then console.warn with a short message and that sanitized object
so no userId or stack traces are logged.
---
Nitpick comments:
In `@packages/blinkcard/docs/classes/CameraManager.md`:
- Around line 260-278: Add a one-line behavioral description to the
addErrorCallback() doc entry (similar to addFrameCaptureCallback) that states
when the provided errorCallback is invoked (on camera/error events) and that the
function returned by addErrorCallback unsubscribes the callback; update the
documentation text around the addErrorCallback signature to include that
sentence so readers understand invocation timing and that the returned () =>
boolean removes the subscription.
In `@packages/blinkid-ux-manager/src/core/test-helpers.integration.ts`:
- Around line 83-86: Add a brief inline comment next to the double type
assertion used when calling createBlinkIdUxManager with scanningSession (e.g.,
`scanningSession as unknown as Parameters<typeof createBlinkIdUxManager>[1]`)
explaining that the double assertion is intentional to bypass strict TypeScript
mismatches in test mocks, that the runtime shape is compatible for the test
harness (cameraHarness.cameraManager), and that this is safe because it’s
limited to integration test utilities; place the comment directly above or
beside the assertion near the createBlinkIdUxManager call so future maintainers
understand why the cast is used.
In `@packages/blinkid/docs/classes/CameraManager.md`:
- Around line 260-278: The documentation for addErrorCallback lacks a
descriptive summary; add a brief sentence or two above the parameters describing
its purpose and invocation conditions (e.g., when camera or streaming errors
occur), mirroring the style used by addFrameCaptureCallback; update the
CameraManager.md addErrorCallback section to state when the ErrorCallback is
called, what kinds of errors are passed, and any return semantics (the boolean
returned by the callback) so consumers understand behavior of addErrorCallback
and its returned () => boolean unsubscription function.
In `@packages/blinkid/src/createBlinkIdUi.test.ts`:
- Around line 339-366: The test "best-effort reports crashes through the core
after session creation" contains a redundant assertion: remove the extraction of
firstPinglet and the expect(firstPinglet?.sessionNumber).toBe(0) check because
sessionNumber is already asserted via expect.objectContaining({ sessionNumber: 0
}) on the mockReportPinglet call; keep the rest of the test (mock setup,
createBlinkId call, mockReportPinglet/sendPinglets/scanningSession assertions)
and delete only the firstPinglet variable extraction and its expect assertion.
In `@packages/blinkid/src/createBlinkIdUi.ts`:
- Around line 203-205: Remove the unreachable null-check for blinkIdCore in
createBlinkIdUi (the if (!blinkIdCore) throw new Error(...) block) because
blinkIdCore is assigned earlier and any failure would already exit to the catch;
simply delete that conditional and its throw; if you want safety retain an
assertion-style comment or use a TypeScript non-null assertion where blinkIdCore
is referenced instead of keeping this dead runtime check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e371f2c-999a-4622-b2a0-91f2f9b6de7b
⛔ Files ignored due to path filters (20)
packages/blinkcard-wasm/dist/advanced-threads/BlinkCardModule.jsis excluded by!**/dist/**packages/blinkcard-wasm/dist/advanced-threads/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/advanced/BlinkCardModule.jsis excluded by!**/dist/**packages/blinkcard-wasm/dist/advanced/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/basic/BlinkCardModule.jsis excluded by!**/dist/**packages/blinkcard-wasm/dist/basic/BlinkCardModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkcard-wasm/dist/size-manifest.jsonis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced-threads/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced-threads/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/full/advanced/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/advanced/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/full/basic/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/full/basic/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/advanced-threads/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/lightweight/advanced-threads/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/advanced/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/lightweight/advanced/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/lightweight/basic/BlinkIdModule.jsis excluded by!**/dist/**packages/blinkid-wasm/dist/lightweight/basic/BlinkIdModule.wasmis excluded by!**/dist/**,!**/*.wasmpackages/blinkid-wasm/dist/size-manifest.jsonis excluded by!**/dist/**
📒 Files selected for processing (77)
.gitignorepackages/analytics/CHANGELOG.mdpackages/analytics/package.jsonpackages/analytics/src/AnalyticService.test.tspackages/analytics/src/AnalyticService.tspackages/blinkcard-core/CHANGELOG.mdpackages/blinkcard-core/docs/classes/BlinkCardWorker.mdpackages/blinkcard-core/package.jsonpackages/blinkcard-ux-manager/CHANGELOG.mdpackages/blinkcard-ux-manager/package.jsonpackages/blinkcard-ux-manager/src/core/BlinkCardUxManager.test.tspackages/blinkcard-ux-manager/src/core/BlinkCardUxManager.tspackages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.tspackages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.tspackages/blinkcard-ux-manager/src/core/test-helpers.integration.tspackages/blinkcard-ux-manager/src/ui/BlinkCardFeedbackUi.tsxpackages/blinkcard-wasm/CHANGELOG.mdpackages/blinkcard-wasm/package.jsonpackages/blinkcard-worker/CHANGELOG.mdpackages/blinkcard-worker/docs/classes/BlinkCardWorker.mdpackages/blinkcard-worker/package.jsonpackages/blinkcard-worker/src/BlinkCardWorker.initBlinkCard.test.tspackages/blinkcard-worker/src/BlinkCardWorker.test.tspackages/blinkcard-worker/src/BlinkCardWorker.tspackages/blinkcard/CHANGELOG.mdpackages/blinkcard/docs/README.mdpackages/blinkcard/docs/classes/BlinkCardWorker.mdpackages/blinkcard/docs/classes/CameraManager.mdpackages/blinkcard/docs/type-aliases/ErrorCallback.mdpackages/blinkcard/package.jsonpackages/blinkcard/src/createBlinkCard.test.tspackages/blinkcard/src/createBlinkCard.tspackages/blinkid-core/CHANGELOG.mdpackages/blinkid-core/docs/classes/BlinkIdWorker.mdpackages/blinkid-core/package.jsonpackages/blinkid-ux-manager/CHANGELOG.mdpackages/blinkid-ux-manager/docs/interfaces/BlinkIdUxManager.mdpackages/blinkid-ux-manager/package.jsonpackages/blinkid-ux-manager/src/core/BlinkIdUxManager.test.tspackages/blinkid-ux-manager/src/core/BlinkIdUxManager.tspackages/blinkid-ux-manager/src/core/createBlinkIdUxManager.test.tspackages/blinkid-ux-manager/src/core/createBlinkIdUxManager.tspackages/blinkid-ux-manager/src/core/test-helpers.integration.tspackages/blinkid-ux-manager/src/ui/BlinkIdFeedbackUi.tsxpackages/blinkid-wasm/CHANGELOG.mdpackages/blinkid-wasm/package.jsonpackages/blinkid-worker/CHANGELOG.mdpackages/blinkid-worker/docs/classes/BlinkIdWorker.mdpackages/blinkid-worker/package.jsonpackages/blinkid-worker/src/BlinkIdWorker.initBlinkId.test.tspackages/blinkid-worker/src/BlinkIdWorker.test.tspackages/blinkid-worker/src/BlinkIdWorker.tspackages/blinkid/CHANGELOG.mdpackages/blinkid/docs/README.mdpackages/blinkid/docs/classes/BlinkIdWorker.mdpackages/blinkid/docs/classes/CameraManager.mdpackages/blinkid/docs/interfaces/BlinkIdUxManager.mdpackages/blinkid/docs/type-aliases/ErrorCallback.mdpackages/blinkid/package.jsonpackages/blinkid/src/createBlinkIdUi.test.tspackages/blinkid/src/createBlinkIdUi.tspackages/camera-manager/CHANGELOG.mdpackages/camera-manager/docs/README.mdpackages/camera-manager/docs/classes/CameraManager.mdpackages/camera-manager/docs/type-aliases/ErrorCallback.mdpackages/camera-manager/package.jsonpackages/camera-manager/src/core/CameraManager.tspackages/core-common/CHANGELOG.mdpackages/core-common/package.jsonpackages/core-common/src/createProxyWorker.tspackages/test-utils/src/mocks/wasmModuleFactory.tspackages/test-utils/src/vitest/cameraManager.test.tspackages/test-utils/src/vitest/cameraManager.tspackages/worker-common/CHANGELOG.mdpackages/worker-common/package.jsonpackages/worker-common/src/workerCrashReporter.test.tspackages/worker-common/src/workerCrashReporter.ts
✅ Files skipped from review due to trivial changes (39)
- packages/camera-manager/docs/README.md
- packages/core-common/CHANGELOG.md
- packages/blinkcard/docs/type-aliases/ErrorCallback.md
- packages/analytics/package.json
- packages/worker-common/CHANGELOG.md
- packages/blinkcard-wasm/CHANGELOG.md
- packages/blinkcard-wasm/package.json
- packages/analytics/CHANGELOG.md
- packages/blinkcard-worker/package.json
- .gitignore
- packages/blinkcard-ux-manager/package.json
- packages/core-common/package.json
- packages/blinkcard/CHANGELOG.md
- packages/blinkcard/docs/README.md
- packages/blinkid/docs/type-aliases/ErrorCallback.md
- packages/blinkid-wasm/package.json
- packages/camera-manager/docs/type-aliases/ErrorCallback.md
- packages/blinkid/docs/README.md
- packages/blinkcard-core/docs/classes/BlinkCardWorker.md
- packages/blinkcard/package.json
- packages/blinkid/package.json
- packages/blinkid-core/package.json
- packages/blinkid-worker/package.json
- packages/blinkcard-core/package.json
- packages/blinkid-ux-manager/package.json
- packages/camera-manager/package.json
- packages/blinkcard-ux-manager/src/ui/BlinkCardFeedbackUi.tsx
- packages/blinkid-wasm/CHANGELOG.md
- packages/blinkid/CHANGELOG.md
- packages/blinkcard-worker/docs/classes/BlinkCardWorker.md
- packages/blinkid-worker/docs/classes/BlinkIdWorker.md
- packages/blinkid-worker/CHANGELOG.md
- packages/blinkcard/docs/classes/BlinkCardWorker.md
- packages/blinkcard-core/CHANGELOG.md
- packages/blinkid-core/CHANGELOG.md
- packages/blinkcard-worker/CHANGELOG.md
- packages/blinkid-ux-manager/src/core/createBlinkIdUxManager.test.ts
- packages/blinkid-ux-manager/CHANGELOG.md
- packages/blinkid-ux-manager/src/core/BlinkIdUxManager.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/blinkid-ux-manager/docs/interfaces/BlinkIdUxManager.md
- packages/blinkid/docs/classes/BlinkIdWorker.md
- packages/blinkid-core/docs/classes/BlinkIdWorker.md
- packages/camera-manager/docs/classes/CameraManager.md
- packages/camera-manager/CHANGELOG.md
- packages/blinkid/docs/interfaces/BlinkIdUxManager.md
- packages/worker-common/package.json
- packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.test.ts
- packages/blinkid-worker/src/BlinkIdWorker.test.ts
- packages/worker-common/src/workerCrashReporter.test.ts
- packages/analytics/src/AnalyticService.test.ts
- packages/analytics/src/AnalyticService.ts
- packages/test-utils/src/mocks/wasmModuleFactory.ts
- packages/camera-manager/src/core/CameraManager.ts
- packages/blinkcard-worker/src/BlinkCardWorker.test.ts
- packages/blinkid-ux-manager/src/core/createBlinkIdUxManager.ts
- packages/worker-common/src/workerCrashReporter.ts
- packages/blinkcard-ux-manager/src/core/test-helpers.integration.ts
| } catch (error) { | ||
| await this.#analytics.logErrorEvent({ | ||
| origin: "ux.frameCapture", | ||
| error, | ||
| errorType: "NonFatal", | ||
| }); | ||
| await this.#analytics.sendPinglets(); | ||
| throw error; |
There was a problem hiding this comment.
Keep telemetry best-effort here so the real SDK error still propagates.
Both catch blocks await analytics work before rethrowing. If logErrorEvent() or sendPinglets() rejects, callers will see the telemetry failure instead of the original process() / getResult() error that actually broke scanning.
Also applies to: 978-987
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkcard-ux-manager/src/core/BlinkCardUxManager.ts` around lines
718 - 725, The catch blocks currently await this.#analytics.logErrorEvent(...)
and this.#analytics.sendPinglets(), which can cause telemetry failures to
replace the original error; change telemetry calls in the catch blocks (where
the caught variable is error) to be best-effort: invoke logErrorEvent and
sendPinglets without letting their failures overwrite the original flow—either
kick them off without awaiting or await them inside their own try/catch and
swallow/log telemetry errors locally—then immediately rethrow the original error
so process()/getResult() callers receive the original exception; specifically
update the catch blocks that call this.#analytics.logErrorEvent and
this.#analytics.sendPinglets (including the other occurrence around lines
978-987) to implement this pattern.
| #handleCameraManagerError = (error: Error) => { | ||
| void this.#analytics.logErrorEvent({ | ||
| origin: "cameraManager.error", | ||
| error, | ||
| errorType: "NonFatal", | ||
| }); | ||
| void this.#analytics.sendPinglets(); | ||
| }; |
There was a problem hiding this comment.
Flush only after the camera-manager error ping is queued.
logErrorEvent() is async, but sendPinglets() is started immediately. That can flush before the ping.error has been enqueued, so some camera-manager failures never leave the queue.
Proposed fix
- `#handleCameraManagerError` = (error: Error) => {
- void this.#analytics.logErrorEvent({
- origin: "cameraManager.error",
- error,
- errorType: "NonFatal",
- });
- void this.#analytics.sendPinglets();
- };
+ `#handleCameraManagerError` = (error: Error) => {
+ void (async () => {
+ try {
+ await this.#analytics.logErrorEvent({
+ origin: "cameraManager.error",
+ error,
+ errorType: "NonFatal",
+ });
+ await this.#analytics.sendPinglets();
+ } catch (reportError) {
+ console.warn(
+ "Failed to report camera manager error pinglet:",
+ reportError,
+ );
+ }
+ })();
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkcard-ux-manager/src/core/BlinkCardUxManager.ts` around lines
733 - 740, The camera-manager error handler starts sendPinglets() before the
async logErrorEvent() finishes, so the flush can occur before the error ping is
enqueued; change the private handler `#handleCameraManagerError` to be async (or
return the log promise) and await this.#analytics.logErrorEvent(...) before
calling this.#analytics.sendPinglets(), ensuring the error ping is queued prior
to flushing (update any callers if they rely on synchronous behavior).
| } catch (error) { | ||
| try { | ||
| await scanningSession.ping({ | ||
| schemaName: "ping.error", | ||
| schemaVersion: "1.0.0", | ||
| data: { | ||
| errorType: "Crash", | ||
| errorMessage: `ux.createBlinkCardUxManager: ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }`, | ||
| stackTrace: error instanceof Error ? error.stack : undefined, | ||
| }, | ||
| }); | ||
| } catch (pingError) { | ||
| console.warn("Failed to report error pinglet:", pingError); | ||
| throw error; | ||
| } | ||
|
|
||
| try { | ||
| await scanningSession.sendPinglets(); | ||
| } catch (sendError) { | ||
| console.warn("Failed to flush error pinglets:", sendError); | ||
| } | ||
|
|
||
| throw error; | ||
| } |
There was a problem hiding this comment.
Control flow prevents sendPinglets from executing when ping fails.
When scanningSession.ping() throws, the throw error on line 71 causes the function to exit immediately, skipping the sendPinglets() call on line 75. If the intent is "best-effort" reporting, sendPinglets should be attempted regardless of whether ping succeeds.
🐛 Proposed fix to ensure sendPinglets is always attempted
} catch (error) {
try {
await scanningSession.ping({
schemaName: "ping.error",
schemaVersion: "1.0.0",
data: {
errorType: "Crash",
errorMessage: `ux.createBlinkCardUxManager: ${
error instanceof Error ? error.message : String(error)
}`,
stackTrace: error instanceof Error ? error.stack : undefined,
},
});
} catch (pingError) {
console.warn("Failed to report error pinglet:", pingError);
- throw error;
}
try {
await scanningSession.sendPinglets();
} catch (sendError) {
console.warn("Failed to flush error pinglets:", sendError);
}
throw error;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| try { | |
| await scanningSession.ping({ | |
| schemaName: "ping.error", | |
| schemaVersion: "1.0.0", | |
| data: { | |
| errorType: "Crash", | |
| errorMessage: `ux.createBlinkCardUxManager: ${ | |
| error instanceof Error ? error.message : String(error) | |
| }`, | |
| stackTrace: error instanceof Error ? error.stack : undefined, | |
| }, | |
| }); | |
| } catch (pingError) { | |
| console.warn("Failed to report error pinglet:", pingError); | |
| throw error; | |
| } | |
| try { | |
| await scanningSession.sendPinglets(); | |
| } catch (sendError) { | |
| console.warn("Failed to flush error pinglets:", sendError); | |
| } | |
| throw error; | |
| } | |
| } catch (error) { | |
| try { | |
| await scanningSession.ping({ | |
| schemaName: "ping.error", | |
| schemaVersion: "1.0.0", | |
| data: { | |
| errorType: "Crash", | |
| errorMessage: `ux.createBlinkCardUxManager: ${ | |
| error instanceof Error ? error.message : String(error) | |
| }`, | |
| stackTrace: error instanceof Error ? error.stack : undefined, | |
| }, | |
| }); | |
| } catch (pingError) { | |
| console.warn("Failed to report error pinglet:", pingError); | |
| } | |
| try { | |
| await scanningSession.sendPinglets(); | |
| } catch (sendError) { | |
| console.warn("Failed to flush error pinglets:", sendError); | |
| } | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkcard-ux-manager/src/core/createBlinkCardUxManager.ts` around
lines 56 - 81, The current catch block rethrows the original error immediately
when scanningSession.ping() fails, which prevents scanningSession.sendPinglets()
from being attempted; change the control flow so sendPinglets() is always
attempted regardless of whether ping() succeeded or threw: capture the original
error in the outer catch (the variable named error), do NOT rethrow inside the
inner catch (pingError), and move the throw of the original error to after the
sendPinglets() attempt (or use a finally that calls
scanningSession.sendPinglets() and then rethrows the original error). Update the
code around scanningSession.ping, the pingError catch, and the
scanningSession.sendPinglets() call to implement this guaranteed "best-effort"
flush behavior.
| if (!this.#wasmModule) { | ||
| return; | ||
| } | ||
|
|
||
| this.reportPinglet({ | ||
| schemaName: "ping.error", | ||
| schemaVersion: "1.0.0", | ||
| sessionNumber: this.#currentSessionNumber, | ||
| data: { | ||
| errorType: "NonFatal", | ||
| errorMessage: | ||
| error instanceof Error ? error.message : String(error), | ||
| stackTrace: error instanceof Error ? error.stack : undefined, | ||
| }, | ||
| }); | ||
| this.sendPinglets(); |
There was a problem hiding this comment.
Don’t return out of terminate() from the session-delete catch.
If session deletion throws after #wasmModule is already gone, this return skips the cleanup block below, so the crash reporter is left installed and self.close() is never called.
Proposed fix
- if (!this.#wasmModule) {
- return;
- }
-
- this.reportPinglet({
- schemaName: "ping.error",
- schemaVersion: "1.0.0",
- sessionNumber: this.#currentSessionNumber,
- data: {
- errorType: "NonFatal",
- errorMessage:
- error instanceof Error ? error.message : String(error),
- stackTrace: error instanceof Error ? error.stack : undefined,
- },
- });
- this.sendPinglets();
+ if (this.#wasmModule) {
+ this.reportPinglet({
+ schemaName: "ping.error",
+ schemaVersion: "1.0.0",
+ sessionNumber: this.#currentSessionNumber,
+ data: {
+ errorType: "NonFatal",
+ errorMessage:
+ error instanceof Error ? error.message : String(error),
+ stackTrace: error instanceof Error ? error.stack : undefined,
+ },
+ });
+ this.sendPinglets();
+ }Also applies to: 723-731
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkcard-worker/src/BlinkCardWorker.ts` around lines 702 - 717, In
terminate(), do not early-return from the session-delete catch when checking
this.#wasmModule — remove the `return` (or move the error-reporting logic out of
the conditional) so that reportPinglet/sendPinglets can run but the cleanup
block (uninstalling crash reporter and calling self.close()) always executes;
alternatively wrap the session deletion and error reporting in a
try/catch/finally so that the final cleanup (uninstall crash reporter, call
self.close()) runs regardless of whether `#wasmModule` is present or session
deletion throws (affecting symbols: terminate(), this.#wasmModule,
reportPinglet, sendPinglets, crash reporter uninstall, self.close()).
| const cameraManager = new CameraManager(); | ||
|
|
||
| // we create the UX manager | ||
| const blinkCardUxManager = await createBlinkCardUxManager( | ||
| cameraManager, | ||
| scanningSession, | ||
| ); | ||
|
|
||
| // this creates the UI and attaches it to the DOM | ||
| const cameraUi = await createCameraManagerUi( | ||
| cameraManager, | ||
| targetNode, | ||
| cameraManagerUiOptions, | ||
| ); |
There was a problem hiding this comment.
Clean up partially initialized resources before rethrowing.
If any step after loadBlinkCardCore() fails, this path reports the pinglet and rethrows, but the caller never gets a destroy() handle. Because cameraManager/cameraUi are scoped inside the try, the partially initialized core/camera stack can leak on init failures.
Proposed fix
export const createBlinkCard = async ({
licenseKey,
microblinkProxyUrl,
targetNode,
cameraManagerUiOptions,
initialMemory,
resourcesLocation,
scanningSettings,
wasmVariant,
feedbackUiOptions,
}: BlinkCardComponentOptions) => {
let blinkCardCore: BlinkCardCore | undefined;
+ let cameraManager: CameraManager | undefined;
+ let blinkCardUxManager: BlinkCardUxManagerType | undefined;
+ let cameraUi: CameraManagerComponent | undefined;
let scanningSession:
| Awaited<ReturnType<BlinkCardCore["createScanningSession"]>>
| undefined;
try {
@@
- const cameraManager = new CameraManager();
+ cameraManager = new CameraManager();
@@
- const blinkCardUxManager = await createBlinkCardUxManager(
+ blinkCardUxManager = await createBlinkCardUxManager(
cameraManager,
scanningSession,
);
@@
- const cameraUi = await createCameraManagerUi(
+ cameraUi = await createCameraManagerUi(
cameraManager,
targetNode,
cameraManagerUiOptions,
@@
} catch (error) {
if (blinkCardCore) {
@@
}
}
+
+ try {
+ blinkCardUxManager?.destroy();
+ cameraUi?.dismount();
+ await blinkCardCore?.terminate();
+ } catch (cleanupError) {
+ console.warn("Failed to clean up BlinkCard after init error:", cleanupError);
+ }
throw error;
}
};Also applies to: 219-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkcard/src/createBlinkCard.ts` around lines 153 - 166, The try
path can leave partially initialized resources (core, cameraManager, cameraUi,
blinkCardUxManager) undisposed if an error is thrown after loadBlinkCardCore();
modify the init flow around loadBlinkCardCore(), cameraManager,
createBlinkCardUxManager, and createCameraManagerUi so that any resources
successfully constructed before the error are properly cleaned up (call their
destroy/cleanup methods) before rethrowing; ensure you capture references to
cameraManager, blinkCardUxManager, cameraUi and the core client in outer scope
so the catch/finally can call destroy() on whichever were created and still
propagate the original error.
| if (!this.#wasmModule) { | ||
| return; | ||
| } | ||
|
|
||
| this.reportPinglet({ | ||
| schemaName: "ping.error", | ||
| schemaVersion: "1.0.0", | ||
| sessionNumber: this.#currentSessionNumber, | ||
| data: { | ||
| errorType: "NonFatal", | ||
| errorMessage: | ||
| error instanceof Error ? error.message : String(error), | ||
| stackTrace: error instanceof Error ? error.stack : undefined, | ||
| }, | ||
| }); | ||
| this.sendPinglets(); |
There was a problem hiding this comment.
Don’t return out of terminate() from this catch.
If delete() throws after #wasmModule has already been cleared, this return skips the shutdown path below, so the crash reporter is never removed and self.close() is never reached.
Proposed fix
- if (!this.#wasmModule) {
- return;
- }
-
- this.reportPinglet({
- schemaName: "ping.error",
- schemaVersion: "1.0.0",
- sessionNumber: this.#currentSessionNumber,
- data: {
- errorType: "NonFatal",
- errorMessage:
- error instanceof Error ? error.message : String(error),
- stackTrace: error instanceof Error ? error.stack : undefined,
- },
- });
- this.sendPinglets();
+ if (this.#wasmModule) {
+ this.reportPinglet({
+ schemaName: "ping.error",
+ schemaVersion: "1.0.0",
+ sessionNumber: this.#currentSessionNumber,
+ data: {
+ errorType: "NonFatal",
+ errorMessage:
+ error instanceof Error ? error.message : String(error),
+ stackTrace: error instanceof Error ? error.stack : undefined,
+ },
+ });
+ this.sendPinglets();
+ }Also applies to: 839-847
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blinkid-worker/src/BlinkIdWorker.ts` around lines 818 - 833, In
terminate() the catch block currently returns when this.#wasmModule is falsy,
which skips the shutdown path (removal of the crash reporter and self.close());
instead remove the early return and let execution continue to the shutdown logic
so the crash reporter is removed and self.close() runs; apply the same change to
the second catch (lines 839-847) so that after reporting the pinglet via
reportPinglet(...) and sendPinglets() the function does not return early even if
this.#wasmModule is missing or delete() threw.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores