Skip to content

Commit

Permalink
Merge pull request #133 from os2display/feature/2484-black-screen-fixes
Browse files Browse the repository at this point in the history
Added retry of loading remote components
  • Loading branch information
tuj authored Sep 20, 2024
2 parents b6f8ecf + 769b5c2 commit d460091
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ end_of_line = LF
insert_final_newline = true
trim_trailing_whitespace = true

[*.{php,html,twig,js,css,scss,json,yaml}]
[*.{php,html,twig,js,css,scss,json,yaml,jsx}]
indent_style = space
indent_size = 2

Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.

## Unreleased

- [#133](https://github.com/os2display/display-client/pull/133)
- Added error message ER201 on screen when remote component could not load.
- Added error timestamp to remote component loader, to force reload on error.
- [#132](https://github.com/os2display/display-client/pull/132)
- Remove token errors after re-login.
- [#131](https://github.com/os2display/display-client/pull/131)
Expand Down
1 change: 1 addition & 0 deletions docs/error-codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
* ER104: Release file could not be loaded.
* ER105: Token is expired.
* ER106: Token is valid but should have been refreshed.
* ER201: Error loading slide template.
16 changes: 16 additions & 0 deletions src/components/region.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ function Region({ region }) {
document.dispatchEvent(slideDoneEvent);
};

/**
* The slide has encountered an error.
*
* @param {object} slideWithError - The slide
*/
const slideError = (slideWithError) => {
// Set error timestamp to force reload.
const slide = slides.find(
(slideElement) => slideElement.executionId === slideWithError.executionId
);
slide.errorTimestamp = new Date().getTime();
slideDone(slideWithError);
};

/**
* Handle region content event.
*
Expand Down Expand Up @@ -174,6 +188,8 @@ function Region({ region }) {
id={currentSlide.executionId}
run={runId}
slideDone={slideDone}
slideError={slideError}
errorTimestamp={currentSlide.errorTimestamp}
key={currentSlide.executionId}
forwardRef={nodeRefs[currentSlide.executionId]}
/>
Expand Down
44 changes: 37 additions & 7 deletions src/components/slide.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { React } from "react";
import { React, useEffect } from "react";
import PropTypes from "prop-types";
import "./slide.scss";
import ErrorBoundary from "./error-boundary";
import { useRemoteComponent } from "../use-remote-component";
import logger from "../logger/logger";

/**
* Slide component.
Expand All @@ -13,11 +14,23 @@ import { useRemoteComponent } from "../use-remote-component";
* @param {number} props.run - Timestamp for when to run the slide.
* @param {Function} props.slideDone - The function to call when the slide is done running.
* @param {React.ForwardRefRenderFunction} props.forwardRef - The ref for the slide.
* @param {string|null} props.errorTimestamp - Slide error timestamp.
* @param {Function} props.slideError - Callback when slide encountered an error.
* @returns {object} - The component.
*/
function Slide({ slide, id, run, slideDone, forwardRef }) {
function Slide({
slide,
id,
run,
slideDone,
forwardRef,
errorTimestamp = null,
slideError,
}) {
const [loading, err, Component] = useRemoteComponent(
slide.templateData.resources.component
// errorTimestamp ensures reload of component in case of error.
slide.templateData.resources.component +
(errorTimestamp !== null ? `?e=${errorTimestamp}` : "")
);

/**
Expand All @@ -26,11 +39,23 @@ function Slide({ slide, id, run, slideDone, forwardRef }) {
* Call slideDone after a timeout to ensure progression.
*/
const handleError = () => {
logger.warn("Slide error boundary triggered.");

setTimeout(() => {
slideDone(slide);
}, 2000);
slideError(slide);
}, 5000);
};

useEffect(() => {
if (err) {
logger.warn("Remote component loading error.");

setTimeout(() => {
slideError(slide);
}, 5000);
}
}, [err]);

return (
<div
id={id}
Expand All @@ -39,8 +64,11 @@ function Slide({ slide, id, run, slideDone, forwardRef }) {
data-run={run}
data-execution-id={slide.executionId}
>
{loading && <div>...</div>}
{!loading && err == null && Component && (
{loading && "..."}
{!loading && err && !Component && (
<h2 className="frontpage-error">ER201</h2>
)}
{!loading && !err && Component && (
<ErrorBoundary errorHandler={handleError}>
<Component
slide={slide}
Expand All @@ -59,6 +87,7 @@ Slide.propTypes = {
id: PropTypes.string.isRequired,
run: PropTypes.string.isRequired,
slideDone: PropTypes.func.isRequired,
slideError: PropTypes.func.isRequired,
slide: PropTypes.shape({
executionId: PropTypes.string,
templateData: PropTypes.shape({
Expand All @@ -69,6 +98,7 @@ Slide.propTypes = {
PropTypes.array,
]).isRequired,
}).isRequired,
errorTimestamp: PropTypes.string,
forwardRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
Expand Down

0 comments on commit d460091

Please sign in to comment.