Skip to content

fix(embed): single, connection-aware submission error across hosted-web flows - #691

Open
Kwame Yeboah (Yeboahmedia) wants to merge 3 commits into
mainfrom
fix/biometric-kyc-upload-error-message
Open

fix(embed): single, connection-aware submission error across hosted-web flows#691
Kwame Yeboah (Yeboahmedia) wants to merge 3 commits into
mainfrom
fix/biometric-kyc-upload-error-message

Conversation

@Yeboahmedia

@Yeboahmedia Kwame Yeboah (Yeboahmedia) commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

When a hosted-web submission fails on a network outage, the on-page error was generic ("Something went wrong") and stacked a new copy on every retry. This surfaces an actionable connection message and shows it only once. Originally scoped to Biometric KYC (per the report); now applied to every hosted-web flow that had the same pattern, with the duplicated code consolidated into one shared module.

Root cause

displayErrorMessage created a new <p> and prepended it to <main> every time. Most flows relied on resetForm() (which clears .validation-message nodes) to remove the previous one — but the id_info skip path in Biometric KYC accepts the selfie via the smart-camera-web.publish handler, which calls handleFormSubmit() without an event and therefore never runs resetForm(). So each failed click stacked another message. Separately, the caught error always mapped to pages.error.generic, which doesn't tell the user the real problem is their connection.

Changes

New shared module packages/embed/src/js/submission-error.js:

  • displayErrorMessage(message) — reuses a single #submission-error-message element (updates text in place, no stacking). Keeps the validation-message class so existing resetForm() cleanup still removes it, disables the class's text-transform so full-sentence copy reads naturally, and no-ops when there is no <main> so it can't mask the original error inside a catch.
  • isNetworkFailure(error) — walks the cause chain for the isNetworkError flag set by fetchWithTimeout, depth-capped so a circular cause chain can't loop forever.
  • submissionErrorMessage(error, translate) — returns pages.error.checkInternet for network drops (or navigator.onLine === false), otherwise pages.error.generic.

Wired into: biometric-kyc, ekyc, basic-kyc, doc-verification, enhanced-document-verification, smartselfie-auth (each now imports the shared helpers and drops its local displayErrorMessage). No new locale keys — pages.error.checkInternet already exists in all three bundled locales.

e-signature is intentionally left unchanged: its only displayErrorMessage use is a checkbox validation already cleared by resetForm(), so it has no stacking bug.

Cypress regression test (cypress/tests/id-info.cy.cjs) forces the upload to fail and clicks "Yes, use this" twice, asserting the descriptive copy appears and exactly one message is shown.

Reviewer feedback

Both prfectionist findings are addressed in the shared module:

  • Infinite-loop risk in the cause-chain walk → depth cap (max 10).
  • Null main → early return guard.

Affected packages

  • packages/web-components
  • packages/embed
  • packages/smart-camera-web
  • example/
  • Tooling / CI / docs

Test plan

  • npx prettier --check on all touched files — clean
  • eslint on touched files — no new violations (added code is clean; pre-existing file errors are unrelated and unchanged)
  • npm run build (embed) — bundles cleanly; verified each entry point's output contains the new single-element + connection-aware logic
  • npm test (embed) — the Cypress binary on this machine fails its own smoke test (bad option: --no-sandbox), so the e2e was not run locally; the spec follows the existing passing id_info patterns and CI will exercise it
  • Manual reproduction: capture selfie, disable network on the review page, click submit repeatedly → previously one "Something went wrong" per click; now a single "check your connection" message.

Changelog

Unreleased entry:

- Embed: when a submission fails, show an actionable "check your connection" message for network drops (instead of the generic error) and update a single on-page message in place rather than stacking a new one on every retry. Applies across the Biometric KYC, eKYC, Basic KYC, Document Verification, Enhanced Document Verification, and SmartSelfie Authentication flows

…pload

When id_info is supplied, the selection and input screens are skipped and
accepting the selfie ("Yes, use this") submits directly via the publish
handler, which calls handleFormSubmit() with no event. That path never runs
resetForm() — which clears prior `.validation-message` nodes — so every failed
submission prepended a brand-new error paragraph, stacking one message per
click.

- Reuse a single #submission-error-message element so repeated failures update
  the message in place instead of stacking.
- Surface the actionable "check your connection" copy for network-level
  failures (offline/timeout, detected via the isNetworkError flag set by
  fetchWithTimeout and walked up the cause chain) instead of the generic
  "Something went wrong".

Adds an id_info regression test covering repeated failed uploads.
@prfectionist

prfectionist Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 85
🧪 PR contains tests
🔒 No security concerns identified
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Infinite Loop Risk

The isNetworkFailure function walks the cause chain without protection against circular references. If an error's cause chain contains a cycle (e.g., error.cause === error), this will loop forever. Consider adding a depth limit or a Set to track visited errors.

function isNetworkFailure(error) {
  let current = error;
  while (current) {
    if (current.isNetworkError === true) return true;
    current = current.cause;
  }
  return false;
}
Null Reference

displayErrorMessage calls document.querySelector('main') without a null check. If main is not present in the DOM at the time of the error (e.g., during a transition or teardown), main.querySelector(...) will throw a TypeError, masking the original error.

const main = document.querySelector('main');

// Reuse a single error element so repeated failed submissions (e.g.
// clicking "Yes, use this" again while offline) update the existing
// message in place instead of stacking a new one on every click.
let p = main.querySelector('#submission-error-message');

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

🔍 Semgrep Security Scan Results

✅ No security findings detected by p/security-audit ruleset.

@prfectionist

prfectionist Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

Extends the biometric-kyc fix to every hosted-web product flow and removes
the duplicated per-entry-point copies by moving the logic into a shared
submission-error.js module used by biometric-kyc, ekyc, basic-kyc,
doc-verification, enhanced-document-verification and smartselfie-auth.

- displayErrorMessage now reuses a single #submission-error-message element
  so repeated failed submissions update the message in place instead of
  stacking a new one per click. Guards against a missing <main> so it can
  never mask the original error inside a catch handler.
- Network-level failures (offline / timeout) show the actionable
  pages.error.checkInternet copy instead of the generic message, via a
  shared submissionErrorMessage() helper. The isNetworkError cause-chain
  walk is depth-capped so a circular cause chain can't loop forever.

Addresses the automated reviewer feedback (cycle guard + null-main guard).
e-signature is left unchanged — its only displayErrorMessage use is a
checkbox validation already cleared by resetForm(), so it has no stacking bug.
@Yeboahmedia Kwame Yeboah (Yeboahmedia) changed the title fix(biometric-kyc): show a single connection error on failed selfie upload fix(embed): single, connection-aware submission error across hosted-web flows Jul 2, 2026
@Yeboahmedia

Copy link
Copy Markdown
Contributor Author

Thanks @prfectionist — both points were useful and are addressed in the new shared submission-error.js:

  • Infinite-loop risk: isNetworkFailure now caps the cause-chain walk at a fixed depth (10), so a circular cause (e.g. error.cause === error) can't spin forever.
  • Null reference: displayErrorMessage early-returns when there is no <main>, so it can never throw a TypeError that masks the original error inside a catch handler.

While here, I also extended the fix beyond Biometric KYC to every hosted-web flow with the same stacking/generic-message pattern and de-duplicated the per-entry-point copies into the shared module.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions Bot added the Stale label Aug 11, 2026
@github-actions

Copy link
Copy Markdown

This branch has been deployed to s3 / cloudfront.

✅ Preview URL for Smart Camera Web:

https://cdn.smileidentity.com/js/preview-fix/biometric-kyc-upload-error-message/smart-camera-web.js

✅ Preview URL for Embed:

https://cdn.smileidentity.com/inline/preview-fix/biometric-kyc-upload-error-message/js/script.min.js

✅ Preview URL for Web Client (Sandbox):

https://d3m7uav8hin8fx.cloudfront.net

✅ Preview URL for Web Client (Production):

https://d301tws8jcas1o.cloudfront.net

@github-actions github-actions Bot removed the Stale label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants