From d6c6b6c33320b2417c0459f6196d7d083c98197f Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Fri, 3 Apr 2020 10:17:24 -0400 Subject: [PATCH] Better error handling and additional logging [WA-128] (#150) --- common/api_adapter.js | 25 ++++++++++++++++++++++++- common/helpers.js | 2 +- martha_v2/martha_v2.js | 15 ++++++++++----- package-lock.json | 28 ++++++++++++++-------------- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/common/api_adapter.js b/common/api_adapter.js index 7d952e5c..3b78b786 100644 --- a/common/api_adapter.js +++ b/common/api_adapter.js @@ -1,4 +1,5 @@ const request = require('superagent'); +const {Response} = require('../common/helpers'); const MAX_RETRY_ATTEMPTS = 5; const INITIAL_BACKOFF_DELAY = 1000; @@ -32,10 +33,32 @@ async function getHeaders(url, authorization) { async function getJsonFrom(url, authorization, retryAttempt = 1, delay = INITIAL_BACKOFF_DELAY) { try { const {body} = await get('get', url, authorization); - return body; + + /* + handle the case when Martha receives empty JSON body. The reason behind forming a response with status 500 + and throwing it in `try` is so that it can be caught locally and be retried. + */ + if (Object.keys(body).length === 0) { + console.log(`Received an empty JSON body while trying to resolve url '${url}'. Attempt ${retryAttempt}. ` + + 'Creating a response with status 500.'); + + const errorMsg = `Something went wrong while trying to resolve url '${url}'. It came back with empty JSON body!`; + const errorResponseObj = { + response: { + status: 500, + text: errorMsg + } + }; + throw new Response(500, errorResponseObj); + } + else { + console.log(`Successfully received response from url '${url}'.`); + return body; + } } catch (error) { // TODO: capture error on lines 56 and 58 in order to give a more detailed idea of // what went wrong where (see https://broadworkbench.atlassian.net/browse/WA-13) + console.log(`Received error for url '${url}'. Attempt ${retryAttempt}.`); console.error(error); if((error.status >= SERVER_ERROR_CODE && error.status <= NETWORK_AUTH_REQ_CODE) || diff --git a/common/helpers.js b/common/helpers.js index 84c999c9..4c1afe64 100644 --- a/common/helpers.js +++ b/common/helpers.js @@ -83,7 +83,7 @@ function dataObjectUriToHttps(dataObjectUri) { }; const output = url.format(resolutionUrlParts); - console.log(`${dataObjectUri} -> ${output}`); + console.log(`Converting DRS URI to HTTPS: ${dataObjectUri} -> ${output}`); return output; } diff --git a/martha_v2/martha_v2.js b/martha_v2/martha_v2.js index 3a6f09a2..87fccb7b 100644 --- a/martha_v2/martha_v2.js +++ b/martha_v2/martha_v2.js @@ -9,16 +9,20 @@ function parseRequest(req) { } } -function maybeTalkToBond(req, provider = BondProviders.default) { - let myPromise; +async function maybeTalkToBond(req, provider = BondProviders.default) { // Currently HCA data access does not require additional credentials. // The HCA checkout buckets allow object read access for GROUP_All_Users@firecloud.org. if (req && req.headers && req.headers.authorization && provider !== BondProviders.HCA) { - myPromise = apiAdapter.getJsonFrom(`${bondBaseUrl()}/api/link/v1/${provider}/serviceaccount/key`, req.headers.authorization); + try { + return await apiAdapter.getJsonFrom(`${bondBaseUrl()}/api/link/v1/${provider}/serviceaccount/key`, req.headers.authorization); + } catch (error) { + console.log(`Received error while fetching service account from Bond for provider '${provider}'.`); + console.error(error); + throw error; + } } else { - myPromise = Promise.resolve(); + return Promise.resolve(); } - return myPromise; } function aggregateResponses(responses) { @@ -59,6 +63,7 @@ function martha_v2_handler(req, res) { res.status(200).send(aggregateResponses(rawResults)); }) .catch((err) => { + console.log('Received error while either contacting Bond or resolving drs url.'); console.error(err); res.status(502).send(err); }); diff --git a/package-lock.json b/package-lock.json index edd9253e..d01b84df 100644 --- a/package-lock.json +++ b/package-lock.json @@ -273,7 +273,7 @@ "@types/events": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz", - "integrity": "sha512-EaObqwIvayI5a8dCzhFrjKzVwKLxjoG9T6Ppd5CEo07LRKfQ8Yokw54r5+Wq7FaBQ+yXRvQAYPrHwya1/UFt9g==", + "integrity": "sha1-KGLz9Yqaf3w+eNefEw3U1xwlwqc=", "dev": true }, "@types/form-data": { @@ -287,7 +287,7 @@ "@types/glob": { "version": "7.1.1", "resolved": "https://registry.npmjs.org/@types/glob/-/glob-7.1.1.tgz", - "integrity": "sha512-1Bh06cbWJUHMC97acuD6UMG29nMt0Aqz1vF3guLfG+kHHJhy3AyohZFFxYk2f7Q1SQIrNwvncxAE0N/9s70F2w==", + "integrity": "sha1-qlmhxuP7xCHgfM0xqUTDDrpSFXU=", "dev": true, "requires": { "@types/events": "*", @@ -298,7 +298,7 @@ "@types/minimatch": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", - "integrity": "sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==", + "integrity": "sha1-PcoOPzOyAPx9ETnAzZbBJoyt/Z0=", "dev": true }, "@types/node": { @@ -1147,7 +1147,7 @@ "ms": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", - "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", + "integrity": "sha1-0J0fNXtEP0kzgqjrPM0YOHKuYAk=", "dev": true } } @@ -1170,7 +1170,7 @@ "deep-extend": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/deep-extend/-/deep-extend-0.6.0.tgz", - "integrity": "sha512-LOHxIOaPYdHlJRtCQfDIVZtfw/ufM8+rVj649RIHzcm/vGwQRXFt6OPqIFWsm2XEMrNIEtWR64sY1LEKD2vAOA==", + "integrity": "sha1-xPp8lUBKF6nD6Mp+FTcxK3NjMKw=", "dev": true }, "defaults": { @@ -1948,7 +1948,7 @@ "is-error": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/is-error/-/is-error-2.2.2.tgz", - "integrity": "sha512-IOQqts/aHWbiisY5DuPJQ0gcbvaLFCa7fBa9xoLfxBZvQ+ZI/Zh9xoI7Gk+G64N0FdK4AbibytHht2tWgpJWLg==", + "integrity": "sha1-wQreGHs8k1EMVHClVngz7iVkmEM=", "dev": true }, "is-extglob": { @@ -1966,7 +1966,7 @@ "is-glob": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/is-glob/-/is-glob-4.0.1.tgz", - "integrity": "sha512-5G0tKtBTFImOqDnLB2hG6Bp2qcKEFduo4tZu9MT/H6NQv/ghhy30o55ufafxJ/LdH79LLs2Kfrn85TLKyA7BUg==", + "integrity": "sha1-dWfb6fL14kZ7x3q4PEopSCQHpdw=", "dev": true, "requires": { "is-extglob": "^2.1.1" @@ -2079,7 +2079,7 @@ "js-yaml": { "version": "3.13.1", "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.13.1.tgz", - "integrity": "sha512-YfbcO7jXDdyj0DGxYVSlSeQNHbD7XPWvrVWeVUujrQEoZzWJIRrCPoyk6kL6IAjAG2IolMK4T0hNUe0HOUs5Jw==", + "integrity": "sha1-r/FRswv9+o5J4F2iLnQV6d+jeEc=", "dev": true, "requires": { "argparse": "^1.0.7", @@ -2583,7 +2583,7 @@ "p-try": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.2.0.tgz", - "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==", + "integrity": "sha1-yyhoVA4xPWHeWPr741zpAE1VQOY=", "dev": true }, "package-json": { @@ -2619,7 +2619,7 @@ "parse-ms": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/parse-ms/-/parse-ms-2.1.0.tgz", - "integrity": "sha512-kHt7kzLoS9VBZfUsiKjv43mr91ea+U05EyKkEtqp7vNbHxmaVuEqN7XxeEVnGrMtYOAxGrDElSi96K7EgO1zCA==", + "integrity": "sha1-NIVlp1PUOR+lJAKZVrFyy3dTCX0=", "dev": true }, "path-exists": { @@ -2677,13 +2677,13 @@ "pify": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/pify/-/pify-4.0.1.tgz", - "integrity": "sha512-uB80kBFb/tfd68bVleG9T5GGsGPjJrLAUpR5PZIrhBnIaRTQRjqdJSsIKkOP6OAIFbj7GOrcudc5pNjZ+geV2g==", + "integrity": "sha1-SyzSXFDVmHNcUCkiJP2MbfQeMjE=", "dev": true }, "pkg-conf": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/pkg-conf/-/pkg-conf-3.1.0.tgz", - "integrity": "sha512-m0OTbR/5VPNPqO1ph6Fqbj7Hv6QU7gR/tQW40ZqrL1rjgCU85W6C1bJn0BItuJqnR98PWzw7Z8hHeChD1WrgdQ==", + "integrity": "sha1-2fnHXqG64Od5OM3gRbJ22sfMaa4=", "dev": true, "requires": { "find-up": "^3.0.0", @@ -2810,7 +2810,7 @@ "rc": { "version": "1.2.8", "resolved": "https://registry.npmjs.org/rc/-/rc-1.2.8.tgz", - "integrity": "sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==", + "integrity": "sha1-zZJL9SAKB1uDwYjNa54hG3/A0+0=", "dev": true, "requires": { "deep-extend": "^0.6.0", @@ -3529,7 +3529,7 @@ "type-fest": { "version": "0.3.1", "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.3.1.tgz", - "integrity": "sha512-cUGJnCdr4STbePCgqNFbpVNCepa+kAVohJs1sLhxzdH+gnEoOd8VhbYa7pD3zZYGiURWM2xzEII3fQcRizDkYQ==", + "integrity": "sha1-Y9ANIE4FlHT+Xht8ARESu9HcKeE=", "dev": true }, "typedarray": {