ampproject / ampproject/amphtml

I2I: Minimal assertion/error infrastructure in Bento

Open
#32,739 1 comment 0 reactions 0 assignees View on GitHub
INTENT TO IMPLEMENT Stale WG: bento WG: performance
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

## Overview

**AMP** uses a web of helpers to manage reporting errors. **User errors** are caused by publishers providing invalid configurations or using components incorrectly. **Dev errors** are issues within AMP code itself, such as a component failing due to a race condition or timeout with the viewer. User errors include a sentinel string of zero-width spaces so they can always be identified by reporting code. AMP pages report errors to a Google-hosted service for aggregation.

**Bento** needs to throw errors for many of the same reasons. Authors using Bento components should not have to run any AMP-centric code for non-AMP pages. It would be nice if AMP pages using Bento components could still distinguish between user/dev errors. Can this be done (well)?

## Challenges

* Bento/Preact code should not have to know anything about AMP error reporting standards
* It's tough for AMP to identify errors when they are captured by the global error handler, as sometimes only the error message string is preserved

## Approach

* Performance WG consensus: Keep user/dev split, potentially compile away user errors
* Publishers/devs could benefit from identifying `UserError` vs `Error`
* Those who don't care aren't impacted
* Possible future if deemed useful: Two different builds
* Preact packages on NPM have `UserError` compiled away
* AMP builds preserve distinction for reporting
* For comparison React core has user/dev asserts (invariant/local errors)

## Alternatives considered

* No user errors in Bento
* **_Pros: super simple, no tie to AMP_**
* **_Cons: AMP pages lose some useful error reporting metadata_**
* User error class/userAssert in Bento
* **_Pros: familiar for current AMP devs, fits in cleanly with existing error reporting, still provides distinction between user errors and Bento internal errors_**
* **_Cons: authors using Bento probably don't know, care about, or use the sentinel_**
* ~~Monkey-patch the error class/helpers when used by AMP pages~~
* _Justin: probably not feasible without multiple build files_
* Make `new Error` a "user error" by default, and have new `BentoError` for internals; AMP can repackage them when present
* **_Pros: preserves user/dev error split, to AMP-only code_**
* **_Cons: devs will probably throw new `Error`s anyway_**
* Allow assertions to use optional `reportError` callback (defaulting to no-op)
* **_Pros: AMP code can provide error reporting handlers, simpler than dev/user logging system_**
* **_Cons: Callers would have to be refactored or helpers duplicated; we can already hook into global async error handler_**
* Other ideas?

Contributor guide

Open the contributing guide

Research direction

Start with the Overview, Challenges, Approach, and Alternatives considered in this issue. Compare the proposed user/dev error split and reporting options, then define what a minimal Bento assertion/error design must preserve without AMP-specific coupling; done requires an agreed approach and scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.