Skip to content

Commit

Permalink
Improved dashboard error msgs when credentials missing (#113)
Browse files Browse the repository at this point in the history
* Fix dashboard error message when credentials are invalid

* Update vitest coverage deps to 2.1
  • Loading branch information
benvinegar authored Nov 16, 2024
1 parent 1157269 commit dd47ecc
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 39 deletions.
2 changes: 1 addition & 1 deletion app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const links: LinksFunction = () => [{ rel: "stylesheet", href: styles }];
export const loader = ({ context, request }: LoaderFunctionArgs) => {
const url = new URL(request.url);
return json({
version: context.cloudflare.env.CF_PAGES_COMMIT_SHA,
version: context.cloudflare?.env?.CF_PAGES_COMMIT_SHA,
origin: url.origin,
url: request.url,
});
Expand Down
72 changes: 50 additions & 22 deletions app/routes/__tests__/dashboard.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @vitest-environment jsdom
import { json } from "@remix-run/node";
import { json, LoaderFunctionArgs } from "@remix-run/node";
import {
vi,
test,
Expand Down Expand Up @@ -36,29 +36,57 @@ describe("Dashboard route", () => {
});

describe("loader", () => {
test("throws an exception if no Cloudflare credentials are provided", async () => {
// empty strings
await expect(
loader({
context: {
analyticsEngine: new AnalyticsEngineAPI(
"testAccountId",
"testApiToken",
),
cloudflare: {
// @ts-expect-error we don't need to provide all the properties of the cloudflare object
env: {
CF_BEARER_TOKEN: "",
CF_ACCOUNT_ID: "",
},
test("throws a 501 Response if no Cloudflare credentials are provided", async () => {
const mockLoaderParams: LoaderFunctionArgs = {
context: {
analyticsEngine: new AnalyticsEngineAPI(
"testAccountId",
"testApiToken",
),
cloudflare: {
// @ts-expect-error we don't need to provide all the properties of the cloudflare object
env: {
CF_ACCOUNT_ID: "",
CF_BEARER_TOKEN: "",
},
},
// @ts-expect-error we don't need to provide all the properties of the request object
request: {
url: "http://localhost:3000/dashboard",
},
}),
).rejects.toThrow("Missing Cloudflare credentials");
},
// @ts-expect-error we don't need to provide all the properties of the request object
request: {
url: "http://localhost:3000/dashboard",
},
};

try {
await loader(mockLoaderParams);
} catch (error) {
expect(error).toBeInstanceOf(Response);
const response = error as Response;
expect(await response.text()).toBe(
"Missing credentials: CF_ACCOUNT_ID is not set.",
);
expect(response.status).toBe(501);
}

// run it again, this time with account ID present, but bearer token absent
mockLoaderParams.context.cloudflare = {
// @ts-expect-error we don't need to provide all the properties of the cloudflare object
env: {
CF_ACCOUNT_ID: "testAccountId",
CF_BEARER_TOKEN: "",
},
};

try {
await loader(mockLoaderParams);
} catch (error) {
expect(error).toBeInstanceOf(Response);
const response = error as Response;
expect(await response.text()).toBe(
"Missing credentials: CF_BEARER_TOKEN is not set.",
);
expect(response.status).toBe(501);
}
});

test("redirects to ?site=siteId if no siteId is provided via query string", async () => {
Expand Down
35 changes: 29 additions & 6 deletions app/routes/dashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import {
import type { LoaderFunctionArgs, MetaFunction } from "@remix-run/cloudflare";
import { json, redirect } from "@remix-run/cloudflare";
import {
isRouteErrorResponse,
useLoaderData,
useNavigation,
useRouteError,
useSearchParams,
} from "@remix-run/react";

Expand Down Expand Up @@ -41,13 +43,16 @@ const MAX_RETENTION_DAYS = 90;

export const loader = async ({ context, request }: LoaderFunctionArgs) => {
// NOTE: probably duped from getLoadContext / need to de-duplicate
if (
!context.cloudflare.env.CF_BEARER_TOKEN ||
!context.cloudflare.env.CF_ACCOUNT_ID
) {
throw new Error("Missing Cloudflare credentials");
if (!context.cloudflare?.env?.CF_ACCOUNT_ID) {
throw new Response("Missing credentials: CF_ACCOUNT_ID is not set.", {
status: 501,
});
}
if (!context.cloudflare?.env?.CF_BEARER_TOKEN) {
throw new Response("Missing credentials: CF_BEARER_TOKEN is not set.", {
status: 501,
});
}

const { analyticsEngine } = context;

const url = new URL(request.url);
Expand Down Expand Up @@ -273,3 +278,21 @@ export default function Dashboard() {
</div>
);
}

export function ErrorBoundary() {
const error = useRouteError();

const errorTitle = isRouteErrorResponse(error) ? error.status : "Error";
const errorBody = isRouteErrorResponse(error)
? error.data
: error instanceof Error
? error.message
: "Unknown error";

return (
<div className="border-2 rounded p-4 bg-card">
<h1 className="text-2xl font-bold">{errorTitle}</h1>
<p className="text-lg">{errorBody}</p>
</div>
);
}
7 changes: 0 additions & 7 deletions load-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@ type GetLoadContext = (args: {

// Shared implementation compatible with Vite, Wrangler, and Cloudflare Pages
export const getLoadContext: GetLoadContext = ({ context }) => {
if (
!context.cloudflare.env.CF_BEARER_TOKEN ||
!context.cloudflare.env.CF_ACCOUNT_ID
) {
throw new Error("Missing Cloudflare credentials");
}

const analyticsEngine = new AnalyticsEngineAPI(
context.cloudflare.env.CF_ACCOUNT_ID,
context.cloudflare.env.CF_BEARER_TOKEN,
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
"@types/ua-parser-js": "^0.7.39",
"@typescript-eslint/eslint-plugin": "^7.13.1",
"@typescript-eslint/parser": "^7.13.1",
"@vitest/coverage-istanbul": "^1.6.0",
"@vitest/coverage-v8": "^1.6.0",
"@vitest/coverage-istanbul": "2.1",
"@vitest/coverage-v8": "2.1",
"autoprefixer": "^10.4.19",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
Expand All @@ -71,7 +71,7 @@
"typescript": "^5.4.5",
"vite": "^5.3.1",
"vite-tsconfig-paths": "^4.3.2",
"vitest": "^1.6.0",
"vitest": "2.1",
"vitest-dom": "^0.1.1",
"wrangler": "^3.61.0"
},
Expand Down

0 comments on commit dd47ecc

Please sign in to comment.