Improve request error handling
See #1532 for more context. - Errored JSON requests will get back the error in JSON instead of using the status text. This seems better to me because it seems more correct to utilize the response body over hijacking the status text. The caller is expecting JSON anyway. Worst of all I never actually set the status text like I thought I did so it wasn't working to begin with. - Allow the update error to propagate for JSON update requests. It was caught to show the error inline instead of an error page when using the update page but for JSON requests it meant there was no error and no error code so it looked like it succeeded. - Make errors for failed requests to GitHub less incomprehensible. Previously they would just be the code which is no context at all.
This commit is contained in:
parent
c7753f2cf9
commit
27ba64c7e4
|
@ -541,10 +541,10 @@ index eab8591492..26668701f7 100644
|
||||||
options.logService.error(`${logPrefix} socketFactory.connect() failed. Error:`);
|
options.logService.error(`${logPrefix} socketFactory.connect() failed. Error:`);
|
||||||
diff --git a/src/vs/server/browser/client.ts b/src/vs/server/browser/client.ts
|
diff --git a/src/vs/server/browser/client.ts b/src/vs/server/browser/client.ts
|
||||||
new file mode 100644
|
new file mode 100644
|
||||||
index 0000000000..4f8543d975
|
index 0000000000..649cf32f0a
|
||||||
--- /dev/null
|
--- /dev/null
|
||||||
+++ b/src/vs/server/browser/client.ts
|
+++ b/src/vs/server/browser/client.ts
|
||||||
@@ -0,0 +1,266 @@
|
@@ -0,0 +1,264 @@
|
||||||
+import { Emitter } from 'vs/base/common/event';
|
+import { Emitter } from 'vs/base/common/event';
|
||||||
+import { URI } from 'vs/base/common/uri';
|
+import { URI } from 'vs/base/common/uri';
|
||||||
+import { localize } from 'vs/nls';
|
+import { localize } from 'vs/nls';
|
||||||
|
@ -720,11 +720,10 @@ index 0000000000..4f8543d975
|
||||||
+ const response = await fetch(normalize(`${options.base}/update/apply`), {
|
+ const response = await fetch(normalize(`${options.base}/update/apply`), {
|
||||||
+ headers: { "content-type": "application/json" },
|
+ headers: { "content-type": "application/json" },
|
||||||
+ });
|
+ });
|
||||||
+ if (response.status !== 200) {
|
|
||||||
+ throw new Error(response.statusText);
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ const json = await response.json();
|
+ const json = await response.json();
|
||||||
|
+ if (response.status !== 200 || json.error) {
|
||||||
|
+ throw new Error(json.error || response.statusText);
|
||||||
|
+ }
|
||||||
+ (services.get(INotificationService) as INotificationService).info(`Updated to ${json.version}`);
|
+ (services.get(INotificationService) as INotificationService).info(`Updated to ${json.version}`);
|
||||||
+ };
|
+ };
|
||||||
+
|
+
|
||||||
|
@ -734,11 +733,10 @@ index 0000000000..4f8543d975
|
||||||
+ const response = await fetch(normalize(`${options.base}/update`), {
|
+ const response = await fetch(normalize(`${options.base}/update`), {
|
||||||
+ headers: { "content-type": "application/json" },
|
+ headers: { "content-type": "application/json" },
|
||||||
+ });
|
+ });
|
||||||
+ if (response.status !== 200) {
|
|
||||||
+ throw new Error("unexpected response");
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ const json = await response.json();
|
+ const json = await response.json();
|
||||||
|
+ if (response.status !== 200 || json.error) {
|
||||||
|
+ throw new Error(json.error || response.statusText);
|
||||||
|
+ }
|
||||||
+ if (json.isLatest) {
|
+ if (json.isLatest) {
|
||||||
+ return;
|
+ return;
|
||||||
+ }
|
+ }
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
{
|
{
|
||||||
"name": "code-server",
|
"name": "code-server",
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
"version": "3.1.1",
|
"version": "3.1.2",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"clean": "ci/clean.sh",
|
"clean": "ci/clean.sh",
|
||||||
"vscode": "ci/vscode.sh",
|
"vscode": "ci/vscode.sh",
|
||||||
|
|
|
@ -87,8 +87,7 @@ export class UpdateHttpProvider extends HttpProvider {
|
||||||
public async getRoot(
|
public async getRoot(
|
||||||
route: Route,
|
route: Route,
|
||||||
request: http.IncomingMessage,
|
request: http.IncomingMessage,
|
||||||
appliedUpdate?: string,
|
errorOrUpdate?: Update | Error,
|
||||||
error?: Error,
|
|
||||||
): Promise<HttpResponse> {
|
): Promise<HttpResponse> {
|
||||||
if (request.headers["content-type"] === "application/json") {
|
if (request.headers["content-type"] === "application/json") {
|
||||||
if (!this.enabled) {
|
if (!this.enabled) {
|
||||||
|
@ -108,8 +107,13 @@ export class UpdateHttpProvider extends HttpProvider {
|
||||||
}
|
}
|
||||||
const response = await this.getUtf8Resource(this.rootPath, "src/browser/pages/update.html")
|
const response = await this.getUtf8Resource(this.rootPath, "src/browser/pages/update.html")
|
||||||
response.content = response.content
|
response.content = response.content
|
||||||
.replace(/{{UPDATE_STATUS}}/, appliedUpdate ? `Updated to ${appliedUpdate}` : await this.getUpdateHtml())
|
.replace(
|
||||||
.replace(/{{ERROR}}/, error ? `<div class="error">${error.message}</div>` : "")
|
/{{UPDATE_STATUS}}/,
|
||||||
|
errorOrUpdate && !(errorOrUpdate instanceof Error)
|
||||||
|
? `Updated to ${errorOrUpdate.version}`
|
||||||
|
: await this.getUpdateHtml(),
|
||||||
|
)
|
||||||
|
.replace(/{{ERROR}}/, errorOrUpdate instanceof Error ? `<div class="error">${errorOrUpdate.message}</div>` : "")
|
||||||
return this.replaceTemplates(route, response)
|
return this.replaceTemplates(route, response)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -186,11 +190,16 @@ export class UpdateHttpProvider extends HttpProvider {
|
||||||
const update = await this.getUpdate()
|
const update = await this.getUpdate()
|
||||||
if (!this.isLatestVersion(update)) {
|
if (!this.isLatestVersion(update)) {
|
||||||
await this.downloadAndApplyUpdate(update)
|
await this.downloadAndApplyUpdate(update)
|
||||||
return this.getRoot(route, request, update.version)
|
return this.getRoot(route, request, update)
|
||||||
}
|
}
|
||||||
return this.getRoot(route, request)
|
return this.getRoot(route, request)
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return this.getRoot(route, request, undefined, error)
|
// For JSON requests propagate the error. Otherwise catch it so we can
|
||||||
|
// show the error inline with the update button instead of an error page.
|
||||||
|
if (request.headers["content-type"] === "application/json") {
|
||||||
|
throw error
|
||||||
|
}
|
||||||
|
return this.getRoot(route, error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -362,7 +371,7 @@ export class UpdateHttpProvider extends HttpProvider {
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!response.statusCode || response.statusCode < 200 || response.statusCode >= 400) {
|
if (!response.statusCode || response.statusCode < 200 || response.statusCode >= 400) {
|
||||||
return reject(new Error(`${response.statusCode || "500"}`))
|
return reject(new Error(`${uri}: ${response.statusCode || "500"}`))
|
||||||
}
|
}
|
||||||
|
|
||||||
resolve(response)
|
resolve(response)
|
||||||
|
|
|
@ -646,11 +646,19 @@ export class HttpServer {
|
||||||
if (code >= HttpCode.ServerError) {
|
if (code >= HttpCode.ServerError) {
|
||||||
logger.error(error.stack)
|
logger.error(error.stack)
|
||||||
}
|
}
|
||||||
const payload = await route.provider.getErrorRoot(route, code, code, e.message)
|
if (request.headers["content-type"] === "application/json") {
|
||||||
write({
|
write({
|
||||||
code,
|
code,
|
||||||
...payload,
|
content: {
|
||||||
})
|
error: e.message,
|
||||||
|
},
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
write({
|
||||||
|
code,
|
||||||
|
...(await route.provider.getErrorRoot(route, code, code, e.message)),
|
||||||
|
})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue