mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 06:54:27 +00:00
fix(services): clear error when Git API returns HTML instead of JSON
Refresh failed with the cryptic "Unexpected token '<', "<!DOCTYPE ..." is not valid JSON" whenever the Git server returned an HTML page (login, SSO redirect, or proxy/error page) on a 2xx/3xx response. safeRequest only threw on status >= 400, so such bodies slipped through and crashed at response.json. Add BaseGitService.parseJson() which detects non-JSON/HTML bodies and throws an actionable message, and use it for all response.json reads in the GitHub and GitLab services. Also harden parseErrorResponse so HTML error pages produce a clear message instead of dumping the raw document. Add tests covering HTML 2xx responses, HTML detection by leading '<', malformed JSON, and HTML error pages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwioG4CNKUBuKiZdowLFWe
This commit is contained in:
parent
49bfe9f4c2
commit
bcc5cda69c
5 changed files with 110 additions and 10 deletions
6
package-lock.json
generated
6
package-lock.json
generated
|
|
@ -1,16 +1,16 @@
|
|||
{
|
||||
"name": "git-file-sync",
|
||||
"version": "1.0.6",
|
||||
"version": "1.1.2",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "git-file-sync",
|
||||
"version": "1.0.6",
|
||||
"version": "1.1.2",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ignore": "^7.0.5",
|
||||
"obsidian": "*"
|
||||
"obsidian": "latest"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "9.30.1",
|
||||
|
|
|
|||
|
|
@ -75,12 +75,47 @@ export abstract class BaseGitService {
|
|||
|
||||
protected abstract addAuthHeader(headers: Record<string, string>): void;
|
||||
|
||||
/**
|
||||
* Safely parses a response body as JSON.
|
||||
*
|
||||
* Some servers/proxies return a 2xx (or redirect) response whose body is an
|
||||
* HTML page — a login screen, an SSO redirect, or a proxy/error page — rather
|
||||
* than JSON. Accessing `response.json` directly then throws a cryptic
|
||||
* "Unexpected token '<', \"<!DOCTYPE ...\" is not valid JSON" error. This helper
|
||||
* detects that case and throws a clear, actionable message instead.
|
||||
*/
|
||||
protected parseJson<T>(response: RequestUrlResponse): T {
|
||||
const contentType = (response.headers?.['content-type'] ?? response.headers?.['Content-Type'] ?? '').toLowerCase();
|
||||
const bodyText = (response.text ?? '').trimStart();
|
||||
const looksLikeHtml = bodyText.startsWith('<') || contentType.includes('html');
|
||||
|
||||
try {
|
||||
const data = response.json as T;
|
||||
if (data === undefined && looksLikeHtml) throw new Error('non-JSON response');
|
||||
return data;
|
||||
} catch (e) {
|
||||
if (looksLikeHtml) {
|
||||
throw new Error(
|
||||
'Expected a JSON response from the Git server but received an HTML page ' +
|
||||
'(likely a login, SSO redirect, or proxy/error page). ' +
|
||||
'Please check the server URL, your access token, and any network proxy or firewall.'
|
||||
);
|
||||
}
|
||||
throw new Error(`Failed to parse the Git server response as JSON: ${e instanceof Error ? e.message : String(e)}`);
|
||||
}
|
||||
}
|
||||
|
||||
protected parseErrorResponse(response: RequestUrlResponse): string {
|
||||
try {
|
||||
const data = response.json as { message?: string; error?: string };
|
||||
return data.message || data.error || JSON.stringify(data);
|
||||
} catch {
|
||||
return response.text || 'Unknown error';
|
||||
const bodyText = (response.text ?? '').trim();
|
||||
const contentType = (response.headers?.['content-type'] ?? response.headers?.['Content-Type'] ?? '').toLowerCase();
|
||||
if (bodyText.startsWith('<') || contentType.includes('html')) {
|
||||
return 'Received an HTML page instead of a JSON error (likely a login, redirect, or proxy/error page). Check the server URL, access token, and network proxy.';
|
||||
}
|
||||
return bodyText || 'Unknown error';
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
|||
try {
|
||||
const url = `${this.getApiUrl(path)}?ref=${branch}`;
|
||||
const response = await this.safeRequest(url, 'GET');
|
||||
const data = response.json as GitHubContentResponse;
|
||||
const data = this.parseJson<GitHubContentResponse>(response);
|
||||
|
||||
return {
|
||||
content: this.decodeContent(data.content, path),
|
||||
|
|
@ -47,14 +47,14 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
|||
};
|
||||
|
||||
const response = await this.safeRequest(url, 'PUT', body);
|
||||
const data = response.json as { content: { path: string, sha: string } };
|
||||
const data = this.parseJson<{ content: { path: string, sha: string } }>(response);
|
||||
return { path: data.content.path, sha: data.content.sha };
|
||||
}
|
||||
|
||||
async listFiles(branch: string, useFilter = true): Promise<string[]> {
|
||||
const url = `https://api.github.com/repos/${this.owner}/${this.repo}/git/trees/${branch}?recursive=1`;
|
||||
const response = await this.safeRequest(url, 'GET');
|
||||
const data = response.json as GitHubTreeResponse;
|
||||
const data = this.parseJson<GitHubTreeResponse>(response);
|
||||
|
||||
if (data.truncated) {
|
||||
logger.warn('GitHub tree result is truncated. Some files might not be shown.');
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
|
|||
try {
|
||||
const url = `${this.getApiUrl(path)}?ref=${branch}`;
|
||||
const response = await this.safeRequest(url, 'GET');
|
||||
const data = response.json as GitLabFileResponse;
|
||||
const data = this.parseJson<GitLabFileResponse>(response);
|
||||
|
||||
return {
|
||||
content: this.decodeContent(data.content, path),
|
||||
|
|
@ -50,7 +50,7 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
|
|||
|
||||
const method = sha ? 'PUT' : 'POST';
|
||||
const response = await this.safeRequest(url, method, body);
|
||||
const data = response.json as GitLabFileResponse;
|
||||
const data = this.parseJson<GitLabFileResponse>(response);
|
||||
return { path: data.file_path };
|
||||
}
|
||||
|
||||
|
|
@ -63,7 +63,7 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
|
|||
while (true) {
|
||||
const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/tree?ref=${branch}&recursive=true&per_page=${perPage}&page=${page}`;
|
||||
const response = await this.safeRequest(url, 'GET');
|
||||
const data = response.json as GitLabTreeItem[];
|
||||
const data = this.parseJson<GitLabTreeItem[]>(response);
|
||||
|
||||
if (!data || data.length === 0) break;
|
||||
|
||||
|
|
|
|||
|
|
@ -41,6 +41,71 @@ describe('BaseGitService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('parseJson with non-JSON (HTML) responses', () => {
|
||||
// Simulates Obsidian's real RequestUrlResponse.json getter, which throws
|
||||
// SyntaxError when the body is not valid JSON (e.g. an HTML page).
|
||||
function htmlResponse(status = 200): RequestUrlResponse {
|
||||
return {
|
||||
status,
|
||||
headers: { 'content-type': 'text/html; charset=utf-8' },
|
||||
text: '<!DOCTYPE html><html><body>Login</body></html>',
|
||||
get json(): unknown {
|
||||
throw new SyntaxError('Unexpected token \'<\', "<!DOCTYPE "... is not valid JSON');
|
||||
},
|
||||
} as unknown as RequestUrlResponse;
|
||||
}
|
||||
|
||||
it('getFile: throws a clear error when the server returns an HTML page (2xx)', async () => {
|
||||
vi.mocked(requestUrl).mockResolvedValue(htmlResponse(200));
|
||||
await expect(service.getFile('test.md', 'main')).rejects.toThrow(/received an HTML page/);
|
||||
// The cryptic JSON parse error must not leak through.
|
||||
await expect(service.getFile('test.md', 'main')).rejects.not.toThrow(/Unexpected token/);
|
||||
});
|
||||
|
||||
it('listFiles: throws a clear error when the server returns an HTML page (2xx)', async () => {
|
||||
vi.mocked(requestUrl).mockResolvedValue(htmlResponse(200));
|
||||
await expect(service.listFiles('main')).rejects.toThrow(/received an HTML page/);
|
||||
});
|
||||
|
||||
it('detects HTML by leading "<" even without an html content-type', async () => {
|
||||
vi.mocked(requestUrl).mockResolvedValue({
|
||||
status: 200,
|
||||
headers: {},
|
||||
text: ' <!DOCTYPE html>...',
|
||||
get json(): unknown {
|
||||
throw new SyntaxError("Unexpected token '<'");
|
||||
},
|
||||
} as unknown as RequestUrlResponse);
|
||||
await expect(service.listFiles('main')).rejects.toThrow(/received an HTML page/);
|
||||
});
|
||||
|
||||
it('reports a generic JSON parse failure for malformed (non-HTML) JSON', async () => {
|
||||
vi.mocked(requestUrl).mockResolvedValue({
|
||||
status: 200,
|
||||
headers: { 'content-type': 'application/json' },
|
||||
text: '{ "tree": [',
|
||||
get json(): unknown {
|
||||
throw new SyntaxError('Unexpected end of JSON input');
|
||||
},
|
||||
} as unknown as RequestUrlResponse);
|
||||
await expect(service.listFiles('main')).rejects.toThrow(/Failed to parse the Git server response as JSON/);
|
||||
});
|
||||
|
||||
it('parseErrorResponse: surfaces a clear message for an HTML error page (>=400)', async () => {
|
||||
vi.mocked(requestUrl).mockResolvedValue({
|
||||
status: 502,
|
||||
headers: { 'content-type': 'text/html' },
|
||||
text: '<!DOCTYPE html><html><body>Bad Gateway</body></html>',
|
||||
get json(): unknown {
|
||||
throw new SyntaxError("Unexpected token '<'");
|
||||
},
|
||||
} as unknown as RequestUrlResponse);
|
||||
await expect(service.listFiles('main')).rejects.toThrow(/Received an HTML page instead of a JSON error/);
|
||||
// The raw HTML document must not be dumped into the message.
|
||||
await expect(service.listFiles('main')).rejects.not.toThrow(/DOCTYPE/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('encodeContent / decodeContent round-trip', () => {
|
||||
it('should correctly encode and decode UTF-8 content', async () => {
|
||||
const original = 'Hello, 世界! 🌍';
|
||||
|
|
|
|||
Loading…
Reference in a new issue