[codex] Structure mobile external link failures (#3363)

Julius Marminge committed 4 months ago

Commit d60f5c61 parent eacceb97 files · +135 −31Browse files

Unified · Split

7 files

apps/mobile/src/features/files/FileMarkdownPreview.tsx +12 −4
apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx +3 −9
apps/mobile/src/features/threads/GitActionProgressOverlay.tsx +3 −2
apps/mobile/src/features/threads/ThreadGitControls.tsx +4 −8
apps/mobile/src/features/threads/git/GitOverviewSheet.tsx +4 −8
apps/mobile/src/lib/openExternalUrl.test.ts +58 −0
apps/mobile/src/lib/openExternalUrl.ts +51 −0

apps/mobile/src/features/files/FileMarkdownPreview.tsx+12−4

@@ -1,12 +1,13 @@
1		−import { useMemo } from "react";
	1	+import { useCallback, useMemo } from "react";
2	2	import {
3	3	Markdown,
4	4	type CustomRenderers,
5	5	type NodeStyleOverrides,
6	6	type PartialMarkdownTheme,
7	7	} from "react-native-nitro-markdown";
8		−import { Linking, ScrollView, Text as NativeText, View } from "react-native";
	8	+import { ScrollView, Text as NativeText, View } from "react-native";
9	9
	10	+import { tryOpenExternalUrl } from "../../lib/openExternalUrl";
10	11	import { MOBILE_TYPOGRAPHY } from "../../lib/typography";
11	12	import { useThemeColor } from "../../lib/useThemeColor";
12	13	import {
@@ -38,7 +39,7 @@ function useMarkdownPreviewStyles(): MarkdownPreviewStyles { · 25 unmodified lines
38	39	<NativeText
39	40	onPress={() => {
40	41	if (href) {
41		− void Linking.openURL(href);
	42	+ void tryOpenExternalUrl(href, "markdown-link");
42	43	}
43	44	}}
44	45	style={{
@@ -143,12 +144,19 @@ function useMarkdownPreviewStyles(): MarkdownPreviewStyles { · 98 unmodified lines
143	144
144	145	export function FileMarkdownPreview(props: { readonly markdown: string }) {
145	146	const styles = useMarkdownPreviewStyles();
	147	+ const onLinkPress = useCallback((href: string) => {
	148	+ void tryOpenExternalUrl(href, "markdown-link");
	149	+ }, []);
146	150
147	151	return (
148	152	<ScrollView className="flex-1 bg-card" contentContainerStyle={{ padding: 18 }}>
149	153	<View className="mx-auto w-full max-w-[760px]">
150	154	{hasNativeSelectableMarkdownText() ? (
151		− <SelectableMarkdownText markdown={props.markdown} textStyle={styles.nativeTextStyle} />
	155	+ <SelectableMarkdownText
	156	+ markdown={props.markdown}
	157	+ onLinkPress={onLinkPress}
	158	+ textStyle={styles.nativeTextStyle}
	159	+ />
152	160	) : (
153	161	<Markdown
154	162	options={{ gfm: true }}

apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx+3−9

@@ -2,14 +2,7 @@ import Stack from "expo-router/stack"; · 1 unmodified line
2	2	import { SymbolView } from "expo-symbols";
3	3	import { useLocalSearchParams, useRouter } from "expo-router";
4	4	import { useCallback, useMemo, useRef, useState } from "react";
5		−import {
6		− ActivityIndicator,
7		− Linking,
8		− Pressable,
9		− ScrollView,
10		− Text as RNText,
11		− View,
12		−} from "react-native";
	5	+import { ActivityIndicator, Pressable, ScrollView, Text as RNText, View } from "react-native";
13	6	import Svg, { Defs, LinearGradient, Rect, Stop } from "react-native-svg";
14	7	import {
15	8	EnvironmentId,
@@ -23,6 +16,7 @@ import { CopyTextButton } from "../../components/CopyTextButton"; · 7 unmodified lines
23	16	import { EmptyState } from "../../components/EmptyState";
24	17	import { LoadingScreen } from "../../components/LoadingScreen";
25	18	import { cn } from "../../lib/cn";
	19	+import { tryOpenExternalUrl } from "../../lib/openExternalUrl";
26	20	import { buildThreadFilesNavigation } from "../../lib/routes";
27	21	import { MOBILE_TYPOGRAPHY } from "../../lib/typography";
28	22	import { useThemeColor } from "../../lib/useThemeColor";
@@ -255,7 +249,7 @@ function FilePreviewHeader(props: { · 226 unmodified lines
255	249	)}
256	250	onPress={() => {
257	251	if (typeof props.externalPreviewUri === "string") {
258		− void Linking.openURL(props.externalPreviewUri);
	252	+ void tryOpenExternalUrl(props.externalPreviewUri, "file-preview");
259	253	}
260	254	}}
261	255	>

apps/mobile/src/features/threads/GitActionProgressOverlay.tsx+3−2

@@ -1,11 +1,12 @@
1	1	import * as Haptics from "expo-haptics";
2	2	import { SymbolView } from "expo-symbols";
3	3	import { useCallback, useEffect, useRef } from "react";
4		−import { ActivityIndicator, Linking, Pressable, View } from "react-native";
	4	+import { ActivityIndicator, Pressable, View } from "react-native";
5	5	import Animated, { FadeIn, FadeOut } from "react-native-reanimated";
6	6	import { useSafeAreaInsets } from "react-native-safe-area-context";
7	7
8	8	import { AppText as Text } from "../../components/AppText";
	9	+import { tryOpenExternalUrl } from "../../lib/openExternalUrl";
9	10	import { useThemeColor } from "../../lib/useThemeColor";
10	11	import type { GitActionProgress } from "../../state/use-vcs-action-state";
11	12
@@ -30,7 +31,7 @@ export function GitActionProgressOverlay(props: { · 18 unmodified lines
30	31
31	32	const handlePress = useCallback(() => {
32	33	if (progress.prUrl) {
33		− void Linking.openURL(progress.prUrl);
	34	+ void tryOpenExternalUrl(progress.prUrl, "pull-request");
34	35	return;
35	36	}
36	37	if (progress.phase === "success" \|\| progress.phase === "error") {

apps/mobile/src/features/threads/ThreadGitControls.tsx+4−8

@@ -13,8 +13,9 @@ import { · 12 unmodified lines
13	13	import { useLocalSearchParams, useRouter } from "expo-router";
14	14	import Stack from "expo-router/stack";
15	15	import { useCallback, useMemo } from "react";
16		−import { Alert, Linking } from "react-native";
	16	+import { Alert } from "react-native";
17	17	import { buildThreadFilesNavigation, buildThreadReviewRoutePath } from "../../lib/routes";
	18	+import { tryOpenExternalUrl } from "../../lib/openExternalUrl";
18	19	import {
19	20	basename,
20	21	getTerminalStatusLabel,
@@ -125,13 +126,8 @@ export function ThreadGitControls(props: { · 104 unmodified lines
125	126	Alert.alert("No open PR", "This branch does not have an open pull request.");
126	127	return;
127	128	}
128		− try {
129		− await Linking.openURL(prUrl);
130		− } catch (error) {
131		− Alert.alert(
132		− "Unable to open PR",
133		− error instanceof Error ? error.message : "An error occurred.",
134		− );
	129	+ if (!(await tryOpenExternalUrl(prUrl, "pull-request"))) {
	130	+ Alert.alert("Unable to open PR", "The pull request could not be opened.");
135	131	}
136	132	}, [gitStatus]);
137	133

apps/mobile/src/features/threads/git/GitOverviewSheet.tsx+4−8

@@ -8,11 +8,12 @@ import type { EnvironmentId, ThreadId } from "@t3tools/contracts"; · 7 unmodified lines
8	8	import { useLocalSearchParams, useRouter } from "expo-router";
9	9	import { SymbolView } from "expo-symbols";
10	10	import { useCallback, useEffect, useMemo } from "react";
11		−import { Alert, Linking, Pressable, ScrollView, View } from "react-native";
	11	+import { Alert, Pressable, ScrollView, View } from "react-native";
12	12	import { useSafeAreaInsets } from "react-native-safe-area-context";
13	13	import { useThemeColor } from "../../../lib/useThemeColor";
14	14
15	15	import { AppText as Text } from "../../../components/AppText";
	16	+import { tryOpenExternalUrl } from "../../../lib/openExternalUrl";
16	17	import { buildThreadReviewRoutePath } from "../../../lib/routes";
17	18	import { useEnvironmentQuery } from "../../../state/query";
18	19	import { useThreadSelection } from "../../../state/use-thread-selection";
@@ -83,13 +84,8 @@ export function GitOverviewSheet() { · 64 unmodified lines
83	84	Alert.alert("No open PR", "This branch does not have an open pull request.");
84	85	return;
85	86	}
86		− try {
87		− await Linking.openURL(prUrl);
88		− } catch (error) {
89		− Alert.alert(
90		− "Unable to open PR",
91		− error instanceof Error ? error.message : "An error occurred.",
92		− );
	87	+ if (!(await tryOpenExternalUrl(prUrl, "pull-request"))) {
	88	+ Alert.alert("Unable to open PR", "The pull request could not be opened.");
93	89	}
94	90	}, [gitStatus.data]);
95	91

apps/mobile/src/lib/openExternalUrl.test.ts+58−0

@@ -0,0 +1,58 @@
	1	+import { Linking } from "react-native";
	2	+import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test";
	3	+
	4	+import { tryOpenExternalUrl } from "./openExternalUrl";
	5	+
	6	+vi.mock("react-native", () => ({
	7	+ Linking: { openURL: vi.fn() },
	8	+}));
	9	+
	10	+const openURL = vi.mocked(Linking.openURL);
	11	+
	12	+beforeEach(() => {
	13	+ vi.clearAllMocks();
	14	+});
	15	+
	16	+afterEach(() => {
	17	+ vi.restoreAllMocks();
	18	+});
	19	+
	20	+describe("tryOpenExternalUrl", () => {
	21	+ it("opens supported URLs", async () => {
	22	+ openURL.mockResolvedValue(undefined);
	23	+
	24	+ await expect(
	25	+ tryOpenExternalUrl("https://github.com/pingdotgg/t3code", "pull-request"),
	26	+ ).resolves.toBe(true);
	27	+ });
	28	+
	29	+ it("logs stable URL context without exposing the opening failure", async () => {
	30	+ const cause = new Error("browser-unavailable-secret-sentinel");
	31	+ openURL.mockRejectedValue(cause);
	32	+ const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);
	33	+
	34	+ await expect(
	35	+ tryOpenExternalUrl("https://github.com/pingdotgg/t3code/pull/1?token=secret", "pull-request"),
	36	+ ).resolves.toBe(false);
	37	+
	38	+ expect(consoleError).toHaveBeenCalledTimes(1);
	39	+ const [message, attributes] = consoleError.mock.calls[0] ?? [];
	40	+ expect(message).toBe("Failed to open pull-request URL with the https scheme.");
	41	+ expect(attributes).toEqual(
	42	+ expect.objectContaining({
	43	+ _tag: "ExternalUrlOpenError",
	44	+ target: "pull-request",
	45	+ scheme: "https",
	46	+ host: "github.com",
	47	+ stack: expect.stringContaining("ExternalUrlOpenError"),
	48	+ }),
	49	+ );
	50	+ expect(attributes).not.toHaveProperty("url");
	51	+ expect(attributes).not.toHaveProperty("cause");
	52	+ const diagnosticText = [message, ...Object.values(attributes as Record<string, unknown>)]
	53	+ .map(String)
	54	+ .join("\n");
	55	+ expect(diagnosticText).not.toContain("token=secret");
	56	+ expect(diagnosticText).not.toContain("browser-unavailable-secret-sentinel");
	57	+ });
	58	+});

apps/mobile/src/lib/openExternalUrl.ts+51−0

@@ -0,0 +1,51 @@
	1	+import * as Schema from "effect/Schema";
	2	+import { Linking } from "react-native";
	3	+
	4	+const ExternalUrlTarget = Schema.Literals(["file-preview", "markdown-link", "pull-request"]);
	5	+
	6	+export type ExternalUrlTarget = typeof ExternalUrlTarget.Type;
	7	+
	8	+export class ExternalUrlOpenError extends Schema.TaggedErrorClass<ExternalUrlOpenError>()(
	9	+ "ExternalUrlOpenError",
	10	+ {
	11	+ target: ExternalUrlTarget,
	12	+ scheme: Schema.String,
	13	+ host: Schema.optional(Schema.String),
	14	+ cause: Schema.Defect(),
	15	+ },
	16	+) {
	17	+ override get message(): string {
	18	+ return `Failed to open ${this.target} URL with the ${this.scheme} scheme.`;
	19	+ }
	20	+}
	21	+
	22	+function externalUrlMetadata(url: string): { readonly scheme: string; readonly host?: string } {
	23	+ try {
	24	+ const parsed = new URL(url);
	25	+ return {
	26	+ scheme: parsed.protocol.replace(/:$/, "") \|\| "unknown",
	27	+ host: parsed.hostname \|\| undefined,
	28	+ };
	29	+ } catch {
	30	+ return {
	31	+ scheme: /^([a-z][a-z\d+.-]*):/i.exec(url)?.[1]?.toLowerCase() ?? "unknown",
	32	+ };
	33	+ }
	34	+}
	35	+
	36	+export async function tryOpenExternalUrl(url: string, target: ExternalUrlTarget): Promise<boolean> {
	37	+ try {
	38	+ await Linking.openURL(url);
	39	+ return true;
	40	+ } catch (cause) {
	41	+ const error = new ExternalUrlOpenError({ target, ...externalUrlMetadata(url), cause });
	42	+ console.error(error.message, {
	43	+ _tag: error._tag,
	44	+ target: error.target,
	45	+ scheme: error.scheme,
	46	+ host: error.host,
	47	+ stack: error.stack,
	48	+ });
	49	+ return false;
	50	+ }
	51	+}