Skip to content

Fix OneSignal init on preview domains and event-upload on_conflict - #197

Open
anchpop wants to merge 1 commit into
mainfrom
claude/quirky-mayer-4gu80v
Open

anchpop wants to merge 1 commit into
mainfrom
claude/quirky-mayer-4gu80v

Conversation

@anchpop

@anchpop anchpop commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two production Sentry issues that a prior automated fix (#181) partially addressed but didn't fully resolve — both still recurring as of today.

  • yap-frontend/index.html: both OneSignal apps are origin-restricted on their dashboard (dev app → localhost:5173, production app → https://yap.town), so OneSignal.init() throws Can only be used on: ... on any other origin no matter which config is picked. Fix OneSignal dev-mode misdetection, event-upload races, and localStorage guard #181 fixed which config the yap-beta.pages.dev preview channel loads (it was matching the isDevelopment substring check on "dev" and using the dev config), but that channel still isn't an allow-listed origin for either app — it just started throwing with the production origin in the message instead of the dev one. Sentry JAVASCRIPT-REACT-3T/3V, 69+ occurrences, still firing through today. Now we skip init() entirely on origins that aren't allow-listed instead of calling something guaranteed to fail.

  • libraries/weapon/src/supabase.rs: the event upload's Prefer: resolution=ignore-duplicates header (added in Fix OneSignal dev-mode misdetection, event-upload races, and localStorage guard #181 to fix a race where a concurrent sync's duplicate row 409'd the whole batch) only takes effect for the constraint PostgREST's implicit ON CONFLICT targets, which defaults to the primary key (id, a bigserial that's always fresh on insert) unless an on_conflict query parameter names a different one. The actual identity constraint on this table is events_unique_stream_device_index (user_id, stream_id, device_id, within_device_events_index), so the header was a no-op — duplicate rows kept getting a 409 ("duplicate key value violates unique constraint \"events_unique_stream_device_index\""). Sentry JAVASCRIPT-REACT-3D, 8 users affected, still recurring after Fix OneSignal dev-mode misdetection, event-upload races, and localStorage guard #181 shipped. Added the on_conflict parameter naming that constraint so the ignore-duplicates resolution actually applies to it — still only skips inserting rows that already exist under that identity, never updates one, preserving the append-only event invariant.

Other open Sentry issues surveyed (push-service AbortErrors, WASM network aborts, "Failed to fetch" against Supabase/the AI backend, an OPFS NotReadableError) are transient/environmental — browser or network failures already caught and logged, not something a code change can unambiguously prevent — so left alone per the "avoid large changes to chase noise" guidance.

Test plan

  • cargo check --package weapon
  • cargo clippy --package weapon (clean)
  • cargo fmt --package weapon -- --check (clean)
  • Manual: confirm no OneSignal init error on a preview (*.pages.dev) deploy, and normal init still fires on yap.town / local dev

🤖 Generated with Claude Code

https://claude.ai/code/session_01XRLrz5gmo8yetkS42yzRPC


Generated by Claude Code

- yap-frontend/index.html: both OneSignal apps are origin-restricted on
  their dashboard (dev to localhost:5173, production to https://yap.town),
  so init() throws "Can only be used on: ..." on any other origin no
  matter which config is picked. A prior fix (#181) corrected which
  config the yap-beta.pages.dev preview channel loads, but that channel
  still isn't an allowed origin for either app, so init kept throwing —
  just with a different target URL in the message (Sentry
  JAVASCRIPT-REACT-3T/3V, 69+ occurrences). Skip init entirely on origins
  that aren't allow-listed instead of calling something guaranteed to fail.

- libraries/weapon/src/supabase.rs: the event upload's
  `Prefer: resolution=ignore-duplicates` header (added in #181) only
  takes effect for the constraint targeted by PostgREST's implicit
  ON CONFLICT, which defaults to the primary key (`id`, always fresh on
  insert) unless an `on_conflict` query parameter says otherwise. The
  actual identity constraint is `events_unique_stream_device_index`, so
  the header was a no-op and duplicate rows kept 409'ing (Sentry
  JAVASCRIPT-REACT-3D, still recurring after #181 shipped). Add the
  on_conflict parameter naming that constraint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XRLrz5gmo8yetkS42yzRPC

anchpop commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

CI's ci check is red, but not because of this PR's diff (yap-frontend/index.html, libraries/weapon/src/supabase.rs).

Failing test: ai-backend::bin/ai-backend tests::tts_endpoints_respond_without_panicking, panicking at yap-ai-backend/src/main.rs:2557 with /tts/google returned 200 OK — the test expects every TTS route to error out before touching the network once provider API keys are stripped from the environment.

Not this PR's: the same test fails identically (same panic, same message) on PR #196, which only touches yap-frontend/index.html and shares no code with yap-ai-backend. Since it reproduces byte-for-byte on an unrelated branch a day apart, it isn't a one-off infra flake, so I didn't spend the one re-run on it.

Root cause: synthesize_checked_bytes (yap-ai-backend/src/main.rs:326) checks the shared TTS cache before trying any provider:

let cache_filename = language_utils::tts_cache_filename(request, &primary);
if let Some(audio) = tts_cache::lookup(http, &cache_filename).await {
    return Ok(Synthesized { audio, verified: true, synthesized: false });
}

tts_cache::lookup (yap-ai-backend/src/tts_cache.rs:70) is an unauthenticated GET against the public yap-tts-cache R2 bucket — reads need no credentials, only writes do (see the module doc). The test's tts_request("bonjour") in French is a very ordinary phrase that's already cached in that public bucket from real traffic, so the cache lookup hits and returns 200 before the handler ever reaches the GOOGLE_CLOUD_API_KEY check the test is trying to exercise. The test's premise ("errors out before reaching the network") predates the cache lookup and no longer holds.

No existing fix to port — I didn't find another open PR touching this test or tts_cache.

Proposed patch (not pushed here — unrelated to this PR's files, so pushing it would widen the diff): make tts_endpoints_respond_without_panicking request a cache-miss-guaranteed phrase instead of a real one, e.g.

let tts_body = serde_json::to_value(TtsRequest {
    text: format!("test-{}", uuid::Uuid::new_v4()),
    ..tts_request("bonjour")
})
.unwrap();

so the cache lookup reliably misses and the handler actually reaches the stripped-credentials path the test means to cover.

Given this is pre-existing and unrelated to the change here, I'll leave yap-frontend/index.html and libraries/weapon/src/supabase.rs as the full scope of this PR.


Generated by Claude Code


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants