From 67361a3186afce759cc22d76f198a97c28724999 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Wed, 10 Dec 2025 12:45:49 +0000 Subject: [PATCH 1/2] adds error boundaries and handling --- ErrorBoundaries.md | 89 +++++++++++++++++++++ src/components/PullRequest.tsx | 2 +- src/components/PullRequestErrorBoundary.tsx | 55 +++++++++++++ src/components/PullRequestList.tsx | 7 +- src/lib/relay/environment.ts | 40 ++++++++- 5 files changed, 188 insertions(+), 5 deletions(-) create mode 100644 ErrorBoundaries.md create mode 100644 src/components/PullRequestErrorBoundary.tsx diff --git a/ErrorBoundaries.md b/ErrorBoundaries.md new file mode 100644 index 0000000..9a48cd6 --- /dev/null +++ b/ErrorBoundaries.md @@ -0,0 +1,89 @@ +Handover + +Thats great Taz, collections look like they work really well for lists of data. However we are still having to props drill data into each of the pull request list cells and in a larger component this can really start scaling poorly, where the root holds all the context for its children. + +And when an error throws it is handled at the root only causing the entire list to fail. + +This is where fragments really come into their own, they allow us to scope the management of data more granularly to the component concerned with it. + +break down the fragment into the component + +```typescript +onst PullRequest = ({ pr }: { pr: PullRequest_pr$key }) => { + const data = useFragment( + graphql` + fragment PullRequest_pr on PullRequest { + id + number + title + url + state + isDraft + createdAt + updatedAt + repository { + name + nameWithOwner + } + baseRefName + headRefName + additions + deletions + reviewDecision + } + `, + pr + ); +``` + +When we do this we also get a more granual approach to error handling. + +So what happens when we things dont go as expected and we get an error in our data? For example, what we don't have permission to access a particular piece of data? How do we handle that? + +With fragments we can now wrap each pull request component in its own boundary. Doing this we can now also adopt a more strict approach to field errors by adapting the `throwOnFieldError` directive. + +```typescript +
+ {pullRequests.map((pr, index) => ( + + + + ))} +
+``` + +Now we are handling any errors in our schema explicitly but also at a granualar level, preserving the integrity of the rest of the list. + +lets throw a field error to see what that looks like. + +```typescript +function injectFieldErrors(response: any): any { + if (!DEMO_FIELD_ERRORS) return response; + + // Check if this is a pull request query response + const pullRequests = response?.data?.viewer?.pullRequests?.nodes; + if (!Array.isArray(pullRequests)) return response; + + const errors: any[] = response.errors || []; + + pullRequests.forEach((pr: any, index: number) => { + // Inject a field error for every 3rd PR's title field + errors.push({ + message: `Demo error: Failed to fetch title for PR #${pr.number}`, + path: ["viewer", "pullRequests", "nodes", index, "title"], + extensions: { + code: "DEMO_ERROR", + }, + }); + // Set the field to null to simulate a field-level error + pr.title = null; + }); + + if (errors.length > 0) { + response.errors = errors; + console.log("Injected field errors:", response.errors); + } + + return response; +} +``` diff --git a/src/components/PullRequest.tsx b/src/components/PullRequest.tsx index f9e7ae1..90860c5 100644 --- a/src/components/PullRequest.tsx +++ b/src/components/PullRequest.tsx @@ -4,7 +4,7 @@ import { PullRequest_pr$key } from "./__generated__/PullRequest_pr.graphql"; const PullRequest = ({ pr }: { pr: PullRequest_pr$key }) => { const data = useFragment( graphql` - fragment PullRequest_pr on PullRequest { + fragment PullRequest_pr on PullRequest @throwOnFieldError { id number title diff --git a/src/components/PullRequestErrorBoundary.tsx b/src/components/PullRequestErrorBoundary.tsx new file mode 100644 index 0000000..6ca6bf6 --- /dev/null +++ b/src/components/PullRequestErrorBoundary.tsx @@ -0,0 +1,55 @@ +import { Component, ReactNode } from "react"; + +interface Props { + children: ReactNode; + prNumber?: number; +} + +interface State { + hasError: boolean; + error?: Error; +} + +export class PullRequestErrorBoundary extends Component { + constructor(props: Props) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, errorInfo: React.ErrorInfo) { + console.error("PullRequest render error:", error, errorInfo); + } + + render() { + if (this.state.hasError) { + return ( +
+
+
+

+ Failed to load pull request + {this.props.prNumber && ` #${this.props.prNumber}`} +

+

+ {this.state.error?.message || "An unexpected error occurred"} +

+
+ +
+
+ ); + } + + return this.props.children; + } +} + diff --git a/src/components/PullRequestList.tsx b/src/components/PullRequestList.tsx index 3234dae..f56ad04 100644 --- a/src/components/PullRequestList.tsx +++ b/src/components/PullRequestList.tsx @@ -1,6 +1,7 @@ import { graphql, useLazyLoadQuery } from "react-relay"; import { type PullRequestListQuery } from "./__generated__/PullRequestListQuery.graphql"; import PullRequest from "./PullRequest"; +import { PullRequestErrorBoundary } from "./PullRequestErrorBoundary"; const PullRequestListQuery = graphql` query PullRequestListQuery($first: Int!) { @@ -58,8 +59,10 @@ export default function PullRequestList({ count = 20 }: PullRequestListProps) { ) : (
- {pullRequests.map((pr) => ( - + {pullRequests.map((pr, index) => ( + + + ))}
)} diff --git a/src/lib/relay/environment.ts b/src/lib/relay/environment.ts index cffd3fd..90c3db6 100644 --- a/src/lib/relay/environment.ts +++ b/src/lib/relay/environment.ts @@ -8,8 +8,42 @@ import { const HTTP_ENDPOINT = "https://api.github.com/graphql"; +// Demo flag: set to true to inject fake field errors for every 3rd PR +const DEMO_FIELD_ERRORS = true; + let clientEnvironment: Environment | null = null; +// Inject fake field errors into pull request responses for demo purposes +function injectFieldErrors(response: any): any { + if (!DEMO_FIELD_ERRORS) return response; + + // Check if this is a pull request query response + const pullRequests = response?.data?.viewer?.pullRequests?.nodes; + if (!Array.isArray(pullRequests)) return response; + + const errors: any[] = response.errors || []; + + pullRequests.forEach((pr: any, index: number) => { + // Inject a field error for every 3rd PR's title field + errors.push({ + message: `Demo error: Failed to fetch title for PR #${pr.number}`, + path: ["viewer", "pullRequests", "nodes", index, "title"], + extensions: { + code: "DEMO_ERROR", + }, + }); + // Set the field to null to simulate a field-level error + pr.title = null; + }); + + if (errors.length > 0) { + response.errors = errors; + console.log("Injected field errors:", response.errors); + } + + return response; +} + export function createRelayEnvironment(accessToken: string): Environment { const fetchFn: FetchFunction = async (request, variables) => { const resp = await fetch(HTTP_ENDPOINT, { @@ -25,7 +59,10 @@ export function createRelayEnvironment(accessToken: string): Environment { }), }); - return await resp.json(); + const json = await resp.json(); + + // Inject fake field errors for demo purposes + return injectFieldErrors(json); }; return new Environment({ @@ -44,4 +81,3 @@ export function getRelayEnvironment(accessToken: string): Environment { export function resetRelayEnvironment(): void { clientEnvironment = null; } - From 168fd05cce83e44974ae60822058a79d8c5eafdf Mon Sep 17 00:00:00 2001 From: David Murphy Date: Wed, 10 Dec 2025 12:49:53 +0000 Subject: [PATCH 2/2] changes the code to only affect the first PR --- ErrorBoundaries.md | 2 ++ src/lib/relay/environment.ts | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ErrorBoundaries.md b/ErrorBoundaries.md index 9a48cd6..cd64499 100644 --- a/ErrorBoundaries.md +++ b/ErrorBoundaries.md @@ -87,3 +87,5 @@ function injectFieldErrors(response: any): any { return response; } ``` + +As you can see here we are now handling an error on our first PR but the integrity of the second and the rest of the list is preserved. diff --git a/src/lib/relay/environment.ts b/src/lib/relay/environment.ts index 90c3db6..c6eb7cb 100644 --- a/src/lib/relay/environment.ts +++ b/src/lib/relay/environment.ts @@ -19,22 +19,25 @@ function injectFieldErrors(response: any): any { // Check if this is a pull request query response const pullRequests = response?.data?.viewer?.pullRequests?.nodes; - if (!Array.isArray(pullRequests)) return response; + if (!Array.isArray(pullRequests) || pullRequests.length === 0) + return response; const errors: any[] = response.errors || []; - pullRequests.forEach((pr: any, index: number) => { - // Inject a field error for every 3rd PR's title field + // Only affect the first PR in the list + const firstPr = pullRequests[0]; + if (firstPr) { + // Inject a field error for the first PR's title field errors.push({ - message: `Demo error: Failed to fetch title for PR #${pr.number}`, - path: ["viewer", "pullRequests", "nodes", index, "title"], + message: `Demo error: Failed to fetch title for PR #${firstPr.number}`, + path: ["viewer", "pullRequests", "nodes", 0, "title"], extensions: { code: "DEMO_ERROR", }, }); // Set the field to null to simulate a field-level error - pr.title = null; - }); + firstPr.title = null; + } if (errors.length > 0) { response.errors = errors;