Skip to content

Commit 18c735e

Browse files
Merge pull request #36 from DeDuckProject/fix/preview-url-resolution
fix: improve previewUrl resolution error messages and add tests
2 parents 6b98796 + 666c42d commit 18c735e

File tree

3 files changed

+109
-17
lines changed

3 files changed

+109
-17
lines changed

packages/action/src/index.ts

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
DEFAULT_TRIGGER,
1414
type GitGlimpseConfig,
1515
} from '@git-glimpse/core';
16+
import { resolveBaseUrl } from './resolve-base-url.js';
1617

1718
function streamCommand(cmd: string, args: string[]): Promise<string> {
1819
return new Promise((resolve, reject) => {
@@ -154,13 +155,12 @@ async function run(): Promise<void> {
154155
return;
155156
}
156157

157-
const baseUrl = resolveBaseUrl(config, previewUrlInput);
158-
if (!baseUrl) {
159-
core.setFailed(
160-
'No base URL available. Set app.previewUrl or app.startCommand + app.readyWhen in config.'
161-
);
158+
const baseUrlResult = resolveBaseUrl(config, previewUrlInput);
159+
if (!baseUrlResult.url) {
160+
core.setFailed(baseUrlResult.error!);
162161
return;
163162
}
163+
const baseUrl = baseUrlResult.url;
164164

165165
if (config.setup) {
166166
core.info(`Running setup: ${config.setup}`);
@@ -225,18 +225,6 @@ async function run(): Promise<void> {
225225
}
226226
}
227227

228-
function resolveBaseUrl(config: GitGlimpseConfig, previewUrlOverride?: string): string | null {
229-
const previewUrl = previewUrlOverride ?? config.app.previewUrl;
230-
if (previewUrl) {
231-
const resolved = process.env[previewUrl] ?? previewUrl;
232-
return resolved.startsWith('http') ? resolved : null;
233-
}
234-
if (config.app.readyWhen?.url) {
235-
const u = new URL(config.app.readyWhen.url);
236-
return u.origin;
237-
}
238-
return 'http://localhost:3000';
239-
}
240228

241229
async function startApp(
242230
startCommand: string,
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import type { GitGlimpseConfig } from '../../core/src/config/schema.js';
2+
3+
export function resolveBaseUrl(
4+
config: GitGlimpseConfig,
5+
previewUrlOverride?: string
6+
): { url: string; error?: never } | { url?: never; error: string } {
7+
const previewUrl = previewUrlOverride ?? config.app.previewUrl;
8+
if (previewUrl) {
9+
const resolved = process.env[previewUrl];
10+
if (resolved === undefined) {
11+
// previewUrl is a literal URL string, not an env var name
12+
if (previewUrl.startsWith('http')) return { url: previewUrl };
13+
return {
14+
error:
15+
`app.previewUrl is set to "${previewUrl}" but it doesn't look like a URL and no env var with that name was found. ` +
16+
`Set it to a full URL (e.g. "https://my-preview.vercel.app") or an env var name that is available in this workflow job.`,
17+
};
18+
}
19+
if (!resolved.startsWith('http')) {
20+
return {
21+
error: `Env var "${previewUrl}" was found but its value "${resolved}" is not a valid URL. Expected a value starting with "http".`,
22+
};
23+
}
24+
return { url: resolved };
25+
}
26+
if (config.app.readyWhen?.url) {
27+
const u = new URL(config.app.readyWhen.url);
28+
return { url: u.origin };
29+
}
30+
return { url: 'http://localhost:3000' };
31+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
2+
import { resolveBaseUrl } from '../../packages/action/src/resolve-base-url.js';
3+
import type { GitGlimpseConfig } from '../../packages/core/src/config/schema.js';
4+
5+
function makeConfig(app: GitGlimpseConfig['app']): GitGlimpseConfig {
6+
return {
7+
app,
8+
llm: { provider: 'anthropic' },
9+
recording: { format: 'gif', maxDuration: 30, viewport: { width: 1280, height: 720 } },
10+
} as unknown as GitGlimpseConfig;
11+
}
12+
13+
describe('resolveBaseUrl', () => {
14+
const originalEnv = process.env;
15+
16+
beforeEach(() => {
17+
process.env = { ...originalEnv };
18+
});
19+
20+
afterEach(() => {
21+
process.env = originalEnv;
22+
});
23+
24+
it('returns URL directly when previewUrl is a literal http URL', () => {
25+
const result = resolveBaseUrl(makeConfig({ previewUrl: 'https://my-preview.vercel.app' }));
26+
expect(result.url).toBe('https://my-preview.vercel.app');
27+
expect(result.error).toBeUndefined();
28+
});
29+
30+
it('resolves previewUrl as env var name when the env var is set', () => {
31+
process.env['VERCEL_PREVIEW_URL'] = 'https://my-preview.vercel.app';
32+
const result = resolveBaseUrl(makeConfig({ previewUrl: 'VERCEL_PREVIEW_URL' }));
33+
expect(result.url).toBe('https://my-preview.vercel.app');
34+
expect(result.error).toBeUndefined();
35+
});
36+
37+
it('returns a descriptive error when env var name is set but env var is missing', () => {
38+
delete process.env['VERCEL_PREVIEW_URL'];
39+
const result = resolveBaseUrl(makeConfig({ previewUrl: 'VERCEL_PREVIEW_URL' }));
40+
expect(result.url).toBeUndefined();
41+
expect(result.error).toMatch(/VERCEL_PREVIEW_URL/);
42+
expect(result.error).toMatch(/env var/);
43+
});
44+
45+
it('returns a descriptive error when env var is set but value is not a URL', () => {
46+
process.env['PREVIEW_URL'] = 'not-a-url';
47+
const result = resolveBaseUrl(makeConfig({ previewUrl: 'PREVIEW_URL' }));
48+
expect(result.url).toBeUndefined();
49+
expect(result.error).toMatch(/PREVIEW_URL/);
50+
expect(result.error).toMatch(/not a valid URL/);
51+
});
52+
53+
it('falls back to localhost when no previewUrl and no readyWhen', () => {
54+
const result = resolveBaseUrl(makeConfig({ startCommand: 'npm run dev' }));
55+
expect(result.url).toBe('http://localhost:3000');
56+
});
57+
58+
it('uses readyWhen.url origin as base when set', () => {
59+
const result = resolveBaseUrl(
60+
makeConfig({ startCommand: 'npm run dev', readyWhen: { url: 'http://localhost:4000/health' } })
61+
);
62+
expect(result.url).toBe('http://localhost:4000');
63+
});
64+
65+
it('previewUrlOverride takes precedence over config', () => {
66+
process.env['OVERRIDE_URL'] = 'https://override.example.com';
67+
const result = resolveBaseUrl(
68+
makeConfig({ previewUrl: 'SOME_OTHER_VAR' }),
69+
'OVERRIDE_URL'
70+
);
71+
expect(result.url).toBe('https://override.example.com');
72+
});
73+
});

0 commit comments

Comments
 (0)