Skip to content

Commit fa3a212

Browse files
minrkminrk
authored
Merge pull request #325 from dtaniwaki/handle-backend-error
Handle store backend errors
2 parents fb152f9 + 61c1278 commit fa3a212

File tree

2 files changed

+86
-69
lines changed

2 files changed

+86
-69
lines changed

‎lib/configproxy.js

+72-69
Original file line numberDiff line numberDiff line change
@@ -378,21 +378,19 @@ class ConfigurableProxy extends EventEmitter {
378378
});
379379
}
380380

381-
targetForReq(req) {
381+
async targetForReq(req) {
382382
var metricsTimerEnd = this.metrics.findTargetForReqSummary.startTimer();
383383
// return proxy target for a given url path
384384
var basePath = this.hostRouting ? "/" + parseHost(req) : "";
385385
var path = basePath + decodeURIComponent(URL.parse(req.url).pathname);
386-
387-
return this._routes.getTarget(path).then(function (route) {
388-
metricsTimerEnd();
389-
if (route) {
390-
return {
391-
prefix: route.prefix,
392-
target: route.data.target,
393-
};
394-
}
395-
});
386+
var route = await this._routes.getTarget(path);
387+
metricsTimerEnd();
388+
if (route) {
389+
return {
390+
prefix: route.prefix,
391+
target: route.data.target,
392+
};
393+
}
396394
}
397395

398396
updateLastActivity(prefix) {
@@ -528,72 +526,77 @@ class ConfigurableProxy extends EventEmitter {
528526
var args = Array.prototype.slice.call(arguments, 1);
529527

530528
// get the proxy target
531-
return this.targetForReq(req).then((match) => {
532-
if (!match) {
533-
that.handleProxyError(404, kind, req, res);
534-
return;
535-
}
536-
537-
if (kind === "web") {
538-
that.emit("proxyRequest", req, res);
539-
} else {
540-
that.emit("proxyRequestWs", req, res, args[2]);
541-
}
542-
var prefix = match.prefix;
543-
var target = match.target;
544-
that.log.debug("PROXY %s %s to %s", kind.toUpperCase(), _logUrl(req.url), target);
545-
if (!that.includePrefix) {
546-
req.url = req.url.slice(prefix.length);
547-
}
529+
return this.targetForReq(req)
530+
.then((match) => {
531+
if (!match) {
532+
that.handleProxyError(404, kind, req, res);
533+
return;
534+
}
548535

549-
target = URL.parse(target);
550-
if (that.options.clientSsl) {
551-
target.key = that.options.clientSsl.key;
552-
target.cert = that.options.clientSsl.cert;
553-
target.ca = that.options.clientSsl.ca;
554-
}
536+
if (kind === "web") {
537+
that.emit("proxyRequest", req, res);
538+
} else {
539+
that.emit("proxyRequestWs", req, res, args[2]);
540+
}
541+
var prefix = match.prefix;
542+
var target = match.target;
543+
that.log.debug("PROXY %s %s to %s", kind.toUpperCase(), _logUrl(req.url), target);
544+
if (!that.includePrefix) {
545+
req.url = req.url.slice(prefix.length);
546+
}
555547

556-
// add config argument
557-
args.push({ target: target });
548+
target = URL.parse(target);
549+
if (that.options.clientSsl) {
550+
target.key = that.options.clientSsl.key;
551+
target.cert = that.options.clientSsl.cert;
552+
target.ca = that.options.clientSsl.ca;
553+
}
558554

559-
// add error handling
560-
args.push(function (e) {
561-
that.handleProxyError(503, kind, req, res, e);
562-
});
555+
// add config argument
556+
args.push({ target: target });
563557

564-
// dispatch the actual method, either:
565-
// - proxy.web(req, res, options, errorHandler)
566-
// - proxy.ws(req, socket, head, options, errorHandler)
567-
that.proxy[kind].apply(that.proxy, args);
558+
// add error handling
559+
args.push(function (e) {
560+
that.handleProxyError(503, kind, req, res, e);
561+
});
568562

569-
// update timestamp on any request/reply data as well (this includes websocket data)
570-
req.on("data", function () {
571-
that.updateLastActivity(prefix);
572-
});
563+
// dispatch the actual method, either:
564+
// - proxy.web(req, res, options, errorHandler)
565+
// - proxy.ws(req, socket, head, options, errorHandler)
566+
that.proxy[kind].apply(that.proxy, args);
573567

574-
res.on("data", function () {
575-
that.updateLastActivity(prefix);
576-
});
568+
// update timestamp on any request/reply data as well (this includes websocket data)
569+
req.on("data", function () {
570+
that.updateLastActivity(prefix);
571+
});
577572

578-
if (kind === "web") {
579-
// update last activity on completion of the request
580-
// only consider 'successful' requests activity
581-
// A flood of invalid requests such as 404s or 403s
582-
// or 503s because the endpoint is down
583-
// shouldn't make it look like the endpoint is 'active'
584-
585-
// we no longer register activity at the *start* of the request
586-
// because at that point we don't know if the endpoint is even available
587-
res.on("finish", function () {
588-
// (don't count redirects...but should we?)
589-
if (res.statusCode < 300) {
590-
that.updateLastActivity(prefix);
591-
} else {
592-
that.log.debug("Not recording activity for status %s on %s", res.statusCode, prefix);
593-
}
573+
res.on("data", function () {
574+
that.updateLastActivity(prefix);
594575
});
595-
}
596-
});
576+
577+
if (kind === "web") {
578+
// update last activity on completion of the request
579+
// only consider 'successful' requests activity
580+
// A flood of invalid requests such as 404s or 403s
581+
// or 503s because the endpoint is down
582+
// shouldn't make it look like the endpoint is 'active'
583+
584+
// we no longer register activity at the *start* of the request
585+
// because at that point we don't know if the endpoint is even available
586+
res.on("finish", function () {
587+
// (don't count redirects...but should we?)
588+
if (res.statusCode < 300) {
589+
that.updateLastActivity(prefix);
590+
} else {
591+
that.log.debug("Not recording activity for status %s on %s", res.statusCode, prefix);
592+
}
593+
});
594+
}
595+
})
596+
.catch(function (e) {
597+
if (res.finished) throw e;
598+
that.handleProxyError(500, kind, req, res, e);
599+
});
597600
}
598601

599602
handleProxyWs(req, socket, head) {

‎test/proxy_spec.js

+14
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,20 @@ describe("Proxy Tests", function () {
379379
.then(done);
380380
});
381381

382+
it("backend error", function (done) {
383+
var proxyPort = 55550;
384+
util
385+
.setupProxy(proxyPort, { errorTarget: "http://127.0.0.1:55565" }, [])
386+
.then(() => r("http://127.0.0.1:" + proxyPort + "/%"))
387+
.then((body) => done.fail("Expected 500"))
388+
.catch((err) => {
389+
expect(err.statusCode).toEqual(500);
390+
expect(err.response.headers["content-type"]).toEqual("text/plain");
391+
expect(err.response.body).toEqual("/%");
392+
})
393+
.then(done);
394+
});
395+
382396
it("Redirect location untouched without rewrite options", function (done) {
383397
var redirectTo = "http://foo.com:12345/whatever";
384398
util

0 commit comments

Comments
 (0)