Skip to content

Commit

Permalink
fix: not all WPT API errors logged (DELO-4614) (#77)
Browse files Browse the repository at this point in the history
* fix: no WPT API errors logged (DELO-4614)

* fix: use the same field for error details for the consistency sake

* test
  • Loading branch information
lukaszczerpak-cloudinary authored Aug 13, 2024
1 parent c5caf7b commit 9fa85b0
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 3 deletions.
3 changes: 2 additions & 1 deletion test/apiTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ describe('API', () => {

expect(log.error).to.be.called;
expect(500).to.equal(spy.getCall(0).args[1].thirdPartyErrorCode);
expect("Unknown WPT issue").to.equal(spy.getCall(0).args[1].thirdPartyErrorBody);

log.error.restore();
});
Expand All @@ -85,7 +86,7 @@ describe('API', () => {
expect(log.warn).to.be.called;
expect(log.error).to.not.be.called;
expect(400).to.equal(spy.getCall(0).args[1].thirdPartyErrorCode);
expect("unknown issue").to.equal(spy.getCall(0).args[1].responseBody);
expect("unknown issue").to.equal(spy.getCall(0).args[1].thirdPartyErrorBody);

log.error.restore();
log.warn.restore();
Expand Down
6 changes: 6 additions & 0 deletions util/strings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const truncateString = (str, lim) =>
str.length > lim ? str.slice(0, lim > 3 ? lim - 3 : lim) + '...' : str;

module.exports = {
truncateString
}
4 changes: 3 additions & 1 deletion wtp/apiCaller.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const logger = require('../logger');
const log = logger.logger;
const resultParser = require('./wtpResultsParser');
const cloudinaryCaller = require('../cloudinary/apiCaller');
const {truncateString} = require('../util/strings');
const RESULTS_URL = 'https://www.webpagetest.org/jsonResult.php';
const RUN_TEST_URL = 'http://www.webpagetest.org/runtest.php';
const GET_TEST_STATUS = 'http://www.webpagetest.org/testStatus.php';
Expand Down Expand Up @@ -80,12 +81,13 @@ const runWtpTest = async (url, mobile, cb) => {
throwHttpErrors: false
};
let response;
let rollBarMsg = {testId: "", analyzedUrl: url, thirdPartyErrorCode: "", file: path.basename((__filename))};
let rollBarMsg = {testId: "", analyzedUrl: url, thirdPartyErrorCode: "", thirdPartyErrorMsg: "", file: path.basename((__filename))};
try {
response = await got(options);
const {statusCode, body} = response;
if (statusCode !== 200) {
rollBarMsg.thirdPartyErrorCode = response.statusCode;
rollBarMsg.thirdPartyErrorBody = body && truncateString(body, 1000) || "";
cb({status: 'error', message: 'WTP returned bad status with url ' + url, error: response.statusMessage, logLevel: logger.LOG_LEVEL_ERROR}, null, response, rollBarMsg);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion wtp/wtpResultsParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const parseTestResults = (testJson) => {
const parseTestResponse = (body, rollBarMsg) => {
if (body.statusText !== 'Ok') {
rollBarMsg.thirdPartyErrorCode = body?.statusCode;
rollBarMsg.responseBody = body.statusText;
rollBarMsg.thirdPartyErrorBody = body.statusText;
logger.warn('WPT returned an error', rollBarMsg);
return {status: 'error', message: 'wpt_failure'}
}
Expand Down

0 comments on commit 9fa85b0

Please sign in to comment.