Add MissingElementError and use it within the Skip Link

[domoscargin]

Closes #4128

Throws an error instead of returning if the linked element is missing.

I considered implementing a separate method to check for this, but that feels a bit needless for Skip Link - it may be necessary for more complex components, though.

There's also an argument for throwing errors within getLinkedElement and getFragmentFromUrl (our error message could potentially be more helpful if we could provide the linkedElementId, for example), but since these get caught by the existing check, I kept things simple.

[github-actions]

📋 Stats

File sizes

File	Size
dist/govuk-frontend-4.6.0.min.css	118.07 KiB
dist/govuk-frontend-4.6.0.min.js	47.1 KiB
dist/govuk-frontend-ie8-4.6.0.min.css	79.27 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js	72.92 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs	68.5 KiB
packages/govuk-frontend/dist/govuk/all.mjs	4 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs	359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js	35.39 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs	5.83 KiB

Modules

File	Size
all.mjs	65.04 KiB
components/accordion/accordion.mjs	20.86 KiB
components/button/button.mjs	3.67 KiB
components/character-count/character-count.mjs	20.46 KiB
components/checkboxes/checkboxes.mjs	4.37 KiB
components/error-summary/error-summary.mjs	4.96 KiB
components/exit-this-page/exit-this-page.mjs	15.42 KiB
components/header/header.mjs	2.3 KiB
components/notification-banner/notification-banner.mjs	3.5 KiB
components/radios/radios.mjs	3.37 KiB
components/skip-link/skip-link.mjs	2.67 KiB
components/tabs/tabs.mjs	8.04 KiB

View stats and visualisations on the review app

---

Action run for e1eff14

[colinrotherham]

There's also an argument for throwing errors within getLinkedElement and getFragmentFromUrl (our error message could potentially be more helpful if we could provide the linkedElementId, for example), but since these get caught by the existing check, I kept things simple.

Not a blocker in the slightest, but knowing what's missing is kinda useful!

Could throw (and catch) an error in this.getLinkedElement() but add to { cause } for the developer tools?

try {
  this.$linkedElement = this.getLinkedElement()
} catch (cause) {
  throw new MissingElementError('The linked HTML element does not exist', {
    cause
  })
}

Would be helpful to see what linkedElementId was for sure

[colinrotherham]

Approved! Added a comment about an error cause

For any error thrown it's good to explain what caused it

[domoscargin]

@colinrotherham Added a commit that addresses your comment. As discussed, I've had to check the type of cause.

[colinrotherham]

Not a blocker, but it's triggered a discussion

To keep file size down, could we use Error { cause } to keep our custom errors to a minimum? Otherwise our published package will grow as custom errors are added

E.g. Using TypeError "…not of the expected type" for null or missing things

Component errors

Here's an example with nested { cause } to make the error more informative:

throw new ComponentError('I has died', {
  cause: new TypeError("Argh, 'ere be no link target #pirates"),
  component: 'Skip link'
}

But using the nested { cause } approach to surface all that useful information. It's handy for monitoring tools like Sentry which links chained errors + stack traces

For extra user friendliness a custom toString() (see below) could log the component name:

ComponentError: "Skip link: I has died"
 → Cause: TypeError: "Argh, 'ere be no link target #pirates"

Suggested change

	export class MissingElementError extends GOVUKFrontendError {
	name = 'MissingElementError'
	}
	export class ComponentError extends GOVUKFrontendError {
	name = 'ComponentError'
	component = 'Component name'
	toString() {
	return `${this.component}: ${this.message}`;
	}
	}

[colinrotherham]

There's also a gotcha

We'd need to confirm Babel transforms or a polyfill for JavaScript built-in: Error: cause

[colinrotherham]

For JavaScript built-in: Error: cause browser support are we happy treating this as optional?

If so, do you mind adding a bit of safety to the GOVUKFrontendError constructor?

i.e. Use feature detection, not a polyfill

export class GOVUKFrontendError extends Error {
  name = 'GOVUKFrontendError'

  /**
   * @param {string} message 
   * @param {ErrorOptions} options
   */
  constructor(message, { cause, ...options }) {
    super(message, options)

    // Add cause property where supported
    if (cause && new Error('test', { cause }).cause === cause) {
      this.cause = cause
    }
  }
}

We'll need to:

1. ESLint ignore "ES2022 Error Cause is forbidden" for feature detection purposes
2. ESLint ignore "ES2018 rest/spread properties are forbidden" if Babel handles it already
3. Triple check my feature detection idea, maybe it needs a try/catch?

For 3) it's mainly because 'cause' in Error.prototype didn't work 🙈

[colinrotherham]

Or we remove { cause } for another day?

[domoscargin]

I'm leaning toward leaving { cause } for now - feels like p'raps we need a bit more thinking about how we make it work and can do that as part of iterating on these errors.

I've pushed a change to use the error messages to convey the error to the user, whaddya reckon?

[colinrotherham]

Very clever, absolutely love it

[colinrotherham]

Brilliant @domoscargin

Some non-blocking things since the last push but approved! Thanks again for making the errors actionable, always good to explain why an error was throw if we can

Appreciate we've used double quotes here but back ticks in #4104 so I've written up some content feedback for later:

• Add MissingElementError and use it within the Skip Link #4177

Error causes

We had some good chats about error causes, especially with monitoring tools like Sentry linking chained errors + stack traces together for inspection

So we should pull out some actions/discussions such as:

1. Using Error cause to reduce custom errors (see comment)
2. Spike Error cause feature detection (see comment)

[colinrotherham]

Not a blocker, but we now say "linked element" in the fallback error but "target selector" here

Suggested change

	throw new TypeError(`Target selector "#${linkedElementId}" not found`)
	throw new TypeError(`Linked element selector "#${linkedElementId}" not found`)

[colinrotherham]

Another non-blocker, but we use error everywhere else

Suggested change

	} catch (err) {
	} catch (error) {