Capture failed turnstile challenges to sentry #738
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const user = await getUser(); | ||
| Sentry.captureEvent({ | ||
| message: 'User failed turnstile challenge when signing up for event.', | ||
| level: Sentry.Severity.Fatal, |
There was a problem hiding this comment.
Det er ikke riktig nivå, dette er maks en error, sannsynligvis bare warning
Fatal ville vært om OWF ikke kjører overhode
There was a problem hiding this comment.
I utgangspunktet er jeg enig i at det burde være warning, men vi vet at i 99.9% av tilfellene som dette skjer så er det ikke meningen at det skal ha skjedd. For hvorfor skal folk gidde å botte når det ikke funker uansett siden vi har turnstile. Så mener det er error i vårt tilfelle her.
There was a problem hiding this comment.
Håpet er ihvertfall at turnstile gjør det vanskelig nok å botte til at ingen gidder. Men har ikke testet selv
There was a problem hiding this comment.
99.9% av tilfellene som dette skjer så er det ikke meningen at det skal ha skjedd
Nei, det er ikke det vi vet.
Dette er ikke en fatal error, og skal ikke behandles som det, det er helt naturlig at Turnstile feiler, det at den noen ganger feiler betyr at den er "working as intended".
Så lenge vi har data-ene i sentry spiller det ikke så stor rolle hvilke nivå det er på, tipper det bare er jeg som faktisk følger med er pr. nå
There was a problem hiding this comment.
Enig i at det ikke er en fatal error. Synes det kan være error sett situasjonen vår.
Mener du det er "helt naturlig" om turnstile i blant feiler når reelle brukere prøver å melde seg på?
There was a problem hiding this comment.
"helt naturlig" om turnstile i blant feiler når reelle brukere prøver å melde seg på
Ja, fordi bot-deteksjon er et vanskelig problem, og jeg antar at de nødvendigvis må ha noen false-positives. Nå er jeg også ikke helt kjent med API-et til turnstile her og det kan godt være at det at den sier feil en gang bare betyr at brukeren må trykke på knappen en gang til eller må flytte på musepekeren litt til for å samle info, idk.
Vi vet jo allerede fra Cloudflare-turnstile stats at den feiler ganske ofte, og kan være måten vi har satt den opp (med at den bare er i en modal) legger opp til at den feiler oftere enn nødvendig.
There was a problem hiding this comment.
Ok sure kjører warning. Men synes personlig det like godt kunne vært error i vårt tilfelle.
| message: 'User failed turnstile challenge when signing up for event.', | ||
| level: Sentry.Severity.Fatal, | ||
| extra: { | ||
| event_id: eventId, |
There was a problem hiding this comment.
Tror det fanges via URL-en allerede
| onError={async (error) => { | ||
| const user = await getUser(); | ||
| Sentry.captureEvent({ | ||
| message: 'User failed turnstile challenge when signing up for event.', |
There was a problem hiding this comment.
Vi burde ha en egen ordbok akkurat for event-domenet med norsk og engelsk "melde seg på" = register eller sign up?
| error: error, | ||
| }, | ||
| user: { | ||
| email: user?.profile.email, |
There was a problem hiding this comment.
E-post er personopplysning (PII), auth0 sub-en er ikke
https://www.datatilsynet.no/rettigheter-og-plikter/personopplysninger/
(Vi mangler personvernerklæring)
Sånn generelt burde vi unngå at PII havner i Sentry, siden vi ikke har koblet f.eks. "slett bruker" funksjonaliteten til det
There was a problem hiding this comment.
Godt poeng. Bytter til auth0-sub'en.
| @@ -44,7 +43,8 @@ const AttendButton: FC<IAttendButtonProps> = (props: IAttendButtonProps) => { | |||
| unwrapResult(action); | |||
| addToast('Du har blitt meldt på arrangementet'); | |||
| } catch (err) { | |||
| addToast(`Noe gikk galt under påmeldelse av arrangement, ERROR: ${err.message}`, { type: 'error' }); | |||
| const errorMessage = err instanceof Error ? err.message : 'Unknown error'; | |||
| @@ -10,10 +10,11 @@ interface ICaptchaModalProps { | |||
| toggleModal: () => void; | |||
| setCaptcha: (token: string | null) => void; | |||
| errorText?: React.ReactNode | string; | |||
| onError: (error: Error) => void; | |||
There was a problem hiding this comment.
React-struktur spørsmål: hvorfor driver du å sender logikk via to nivåer? Syntes det bare kompliserer kontroll-flyten.
Ser dette ganske ofte, og syntes det fremstår som et veldig anti-pattern?
There was a problem hiding this comment.
Enig i at flyten er litt komplisert her. Godt mulig at det kan gjøres bedre. Er fort en case av premature optimalization. Sånn det er nå er ikke CaptchaModal knyttet til event-påmelding. Det er en reusable komponent som potensielt kan brukes andre steder. Så da blir det jo sånn at event handlers og tekst som kan være forskjellig fra case til case må være i parent komponenten.
IMO kunne hele greia vært i AttendButton istedenfor at det er en egen komponent. Eller bare gjort den helt spesifikk for event registration.
Synes forøvrig setCaptcha er et dårlig navn her da, som gjør det litt ekstra forvirrende. Kunne vært onValidCaptchaToken e.l.
There was a problem hiding this comment.
Synes det ble veldig rotete med feilmeldingene fra parent component og fra Turnstile onError her da. Men siden vi ikke har store ambisjoner for videreutvikling på owf tenker jeg at sånne ting er greit. As long as it works og ikke er helt uforståelig liksom
|
OWF_SENTRY_DSN var kalt OW4_SENTRY_DSN i doppler 😵💫