Skip to content

Commit

Permalink
fix: optimized tests to run in parallel, removed delay in favor of na…
Browse files Browse the repository at this point in the history
…tive nodejs timers, fixed 404 status code to 400 for app/models pre save and pre validate hooks, prevent google chrome processes in test env and cleanup with browser/page close, sync locales
  • Loading branch information
titanism committed Aug 27, 2024
1 parent 62f12d5 commit 8083077
Show file tree
Hide file tree
Showing 70 changed files with 4,671 additions and 520 deletions.
2 changes: 1 addition & 1 deletion .env.defaults
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ API_RATELIMIT_WHITELIST=138.197.213.185,104.248.224.170
#########
APP_NAME="Forward Email"
APP_COLOR=#20C1ED
TRANSPORT_DEBUG=true
TRANSPORT_DEBUG=false
SEND_EMAIL=false
PREVIEW_EMAIL=true
# openssl rand -base64 32 | tr -d /=+ | cut -c -32
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/stripe.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
* SPDX-License-Identifier: BUSL-1.1
*/

const { setTimeout } = require('node:timers/promises');
const Boom = require('@hapi/boom');
const Stripe = require('stripe');
const _ = require('lodash');
const dayjs = require('dayjs-with-plugins');
const delay = require('delay');
const humanize = require('humanize-string');
const isSANB = require('is-string-and-not-blank');
const ms = require('ms');
Expand Down Expand Up @@ -325,7 +325,7 @@ async function processEvent(ctx, event) {
});
if (!user) throw new Error('User did not exist for customer');
// artificially wait 5s for refund to process
await delay(ms('15s'));
await setTimeout(ms('15s'));
//
// NOTE: this re-uses the payment intent mapper that is also used
// in the job for `sync-stripe-payments` which syncs payments
Expand Down
18 changes: 11 additions & 7 deletions app/controllers/web/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ async function api(ctx) {

Object.assign(ctx.state.meta, data);

let html = pug
const html = pug
.renderFile(filePath, ctx.state)
.replace(new RE2(/BASE_URI/g), config.urls.api)
.replace(new RE2(/AMP4EMAIL/g), 'amp4email')
Expand All @@ -79,14 +79,18 @@ async function api(ctx) {
.replace(new RE2(/ALIAS_ID/g), ':alias_id')
.replace(new RE2(/MEMBER_ID/g), ':member_id');

if (ctx.isAuthenticated())
html = html.replace(
new RE2(/API_TOKEN/g),
ctx.state.user[config.userFields.apiToken]
);
if (!ctx.isAuthenticated()) {
ctx.body = html;
return;
}

// expose it to the view
const dom = new JSDOM(html);
const dom = new JSDOM(
html.replace(
new RE2(/API_TOKEN/g),
ctx.state.user[config.userFields.apiToken]
)
);
const $codeTags = dom.window.document.querySelectorAll(
'code.hljs.language-sh'
);
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/web/my-account/change-modulus-length.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ async function changeModulusLength(ctx) {
const domain = await Domains.findById(ctx.state.domain._id);
if (!domain)
return ctx.throw(
Boom.notFound(ctx.translateError('DOMAIN_DOES_NOT_EXIST'))
Boom.badRequest(ctx.translateError('DOMAIN_DOES_NOT_EXIST'))
);

const redirectTo = ctx.state.l(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ async function createCatchAllPassword(ctx) {
'+tokens +tokens.description +tokens.hash +tokens.salt'
);
if (!domain)
throw Boom.notFound(ctx.translateError('DOMAIN_DOES_NOT_EXIST'));
throw Boom.badRequest(ctx.translateError('DOMAIN_DOES_NOT_EXIST'));

// domain cannot have more than 10 at once
if (Array.isArray(domain.tokens) && domain.tokens.length >= 10)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/web/my-account/retrieve-domain-billing.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

const punycode = require('node:punycode');

const { setTimeout } = require('node:timers/promises');
const Boom = require('@hapi/boom');
const Stripe = require('stripe');
const _ = require('lodash');
const numeral = require('numeral');
const dayjs = require('dayjs-with-plugins');
const delay = require('delay');
const isFQDN = require('is-fqdn');
const isSANB = require('is-string-and-not-blank');
const ms = require('ms');
Expand Down Expand Up @@ -1234,7 +1234,7 @@ async function retrieveDomainBilling(ctx) {
// - and 15s was tested and worked (seemingly) reliably
// (if we have anything more than 15-20s it seems we may get Timeout error)
//
await delay(ms('15s'));
await setTimeout(ms('15s'));

// attempt to lookup the transactions
let transactionId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function updateRestrictedAliasNames(ctx, next) {
ctx.state.domain = await Domains.findById(ctx.state.domain._id);
if (!ctx.state.domain)
return ctx.throw(
Boom.notFound(ctx.translateError('DOMAIN_DOES_NOT_EXIST'))
Boom.badRequest(ctx.translateError('DOMAIN_DOES_NOT_EXIST'))
);

if (isSANB(ctx.request.body.restricted_alias_names)) {
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/web/onboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ async function onboard(ctx, next) {
locale: ctx.locale
});
}
} else if (!ctx.isAuthenticated()) {
} else {
const query = {
email: ctx.request.body.email,
locale: ctx.locale
Expand Down
10 changes: 6 additions & 4 deletions app/models/aliases.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,12 @@ Aliases.pre('save', async function (next) {
]);

if (!domain)
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', alias.locale)
);

if (!user && !alias.is_new_user)
throw Boom.notFound(i18n.translateError('INVALID_USER', alias.locale));
throw Boom.badRequest(i18n.translateError('INVALID_USER', alias.locale));

// filter out a domain's members without actual users
domain.members = domain.members.filter(
Expand Down Expand Up @@ -550,7 +550,9 @@ Aliases.pre('save', async function (next) {
// but populate above is trying to populate it, we can assume it's new
//
if (!member)
throw Boom.notFound(i18n.translateError('INVALID_MEMBER', alias.locale));
throw Boom.badRequest(
i18n.translateError('INVALID_MEMBER', alias.locale)
);

const string = alias.name.replace(/[^\da-z]/g, '');

Expand All @@ -564,7 +566,7 @@ Aliases.pre('save', async function (next) {
) {
const existingAlias = await alias.constructor.findById(alias._id);
if (!existingAlias)
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('ALIAS_DOES_NOT_EXIST', alias.locale)
);
if (existingAlias.name !== alias.name)
Expand Down
20 changes: 10 additions & 10 deletions app/models/domains.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ const punycode = require('node:punycode');
const { Buffer } = require('node:buffer');
const { promisify } = require('node:util');

const { setTimeout } = require('node:timers/promises');
const Boom = require('@hapi/boom');
const RE2 = require('re2');
const _ = require('lodash');
const bytes = require('bytes');
const cryptoRandomString = require('crypto-random-string');
const dayjs = require('dayjs-with-plugins');
const delay = require('delay');
const getDmarcRecord = require('mailauth/lib/dmarc/get-dmarc-record');
const isBase64 = require('is-base64');
const isFQDN = require('is-fqdn');
Expand Down Expand Up @@ -1310,7 +1310,7 @@ async function verifySMTP(domain, resolver, purgeCache = true) {
records
});
// Wait one second for DNS changes to propagate
await delay(ms('1s'));
await setTimeout(ms('1s'));
} catch (err) {
err.domain = domain;
err.records = records;
Expand Down Expand Up @@ -1524,7 +1524,7 @@ async function getVerificationResults(domain, resolver, purgeCache = false) {
types: CACHE_TYPES
});
// Wait one second for DNS changes to propagate
await delay(ms('1s'));
await setTimeout(ms('1s'));
} catch (err) {
err.domain = domain;
logger.error(err);
Expand Down Expand Up @@ -2218,13 +2218,13 @@ async function getMaxQuota(_id, locale = i18n.config.defaultLocale) {
.exec();

if (!domain) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);
}

if (!domain)
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);

Expand Down Expand Up @@ -2265,7 +2265,7 @@ async function getMaxQuota(_id, locale = i18n.config.defaultLocale) {
);

if (adminMembers.length === 0) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);
}
Expand Down Expand Up @@ -2307,7 +2307,7 @@ async function getStorageUsed(_id, _locale, aliasesOnly = false) {
.exec();

if (!domain) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError(
'DOMAIN_DOES_NOT_EXIST_ANYWHERE',
_locale || i18n.config.defaultLocale
Expand Down Expand Up @@ -2354,7 +2354,7 @@ async function getStorageUsed(_id, _locale, aliasesOnly = false) {
);

if (adminMembers.length === 0) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);
}
Expand All @@ -2380,7 +2380,7 @@ async function getStorageUsed(_id, _locale, aliasesOnly = false) {

// Safeguard
if (domainIds.length === 0) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);
}
Expand Down Expand Up @@ -2423,7 +2423,7 @@ async function getStorageUsed(_id, _locale, aliasesOnly = false) {
(typeof results[0] !== 'object' ||
typeof results[0].storage_used !== 'number'))
) {
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST_ANYWHERE', locale)
);
}
Expand Down
12 changes: 7 additions & 5 deletions app/models/emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ Emails.post('save', async function (email) {

const domain = await Domains.findById(email.domain);
if (!domain)
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('DOMAIN_DOES_NOT_EXIST', i18n.config.defaultLocale)
);

Expand Down Expand Up @@ -982,10 +982,10 @@ Emails.statics.queue = async function (
: null);

if (!domain)
throw Boom.notFound(i18n.translateError('DOMAIN_DOES_NOT_EXIST', locale));
throw Boom.badRequest(i18n.translateError('DOMAIN_DOES_NOT_EXIST', locale));

if (domain.is_global)
throw Boom.notFound(
throw Boom.badRequest(
i18n.translateError('EMAIL_SMTP_GLOBAL_NOT_PERMITTED', locale)
);

Expand Down Expand Up @@ -1035,7 +1035,7 @@ Emails.statics.queue = async function (
);

if (!member)
throw Boom.notFound(i18n.translateError('INVALID_MEMBER', locale));
throw Boom.badRequest(i18n.translateError('INVALID_MEMBER', locale));

//
// ensure the alias exists (if it was not a catch-all)
Expand Down Expand Up @@ -1068,7 +1068,9 @@ Emails.statics.queue = async function (

// alias must be enabled
if (!alias.is_enabled)
throw Boom.notFound(i18n.translateError('ALIAS_IS_NOT_ENABLED', locale));
throw Boom.badRequest(
i18n.translateError('ALIAS_IS_NOT_ENABLED', locale)
);
}

// parse the date for SMTP queuing
Expand Down
3 changes: 2 additions & 1 deletion app/models/payments.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const wkhtmltopdf = require('wkhtmltopdf');
mongoose.Error.messages = require('@ladjs/mongoose-error-messages');

const config = require('#config');
const env = require('#config/env');
const i18n = require('#helpers/i18n');

const inline = pify(webResourceInliner.html);
Expand Down Expand Up @@ -532,7 +533,7 @@ async function getPDFReceipt(
});

const options = {
debug: config.env !== 'production',
debug: !env.AXE_SILENT,
pageSize: 'letter',
background: false,
imageDpi: 600,
Expand Down
7 changes: 4 additions & 3 deletions ava.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
* SPDX-License-Identifier: BUSL-1.1
*/

const isCI = require('is-ci');
// const isCI = require('is-ci');
// const { familySync, GLIBC } = require('detect-libc');

module.exports = {
verbose: true,
failFast: true,
serial: true,
// serial: true,
// concurrency: 2, // <--- this defaults to `2` in CI environments
files: ['test/*.js', 'test/**/*.js', 'test/**/**/*.js', '!test/utils.js'],
// <https://github.com/lovell/sharp/issues/3164#issuecomment-1168328811>
// workerThreads: familySync() !== GLIBC,

// <https://github.com/JCMais/node-libcurl/issues/414>
// workerThreads: false,

timeout: isCI ? '1m' : '30s'
timeout: '1m'
};
36 changes: 30 additions & 6 deletions config/alternatives.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ const parseRootDomain = require('#helpers/parse-root-domain');
// TODO: make note that the API must be able to send email
// TODO: outbound smtp monthly limit
// TODO: webhooks
// TODO: CardDAV/CalDAV
// TODO: inbound limit
// TODO: webmail
// TODO: desktop app
// TODO: mobile app
// TODO: iOS Push Notification support (via XAPPLEPUSHSERVICE capability)
// TODO: regex-based aliases
// TODO: calendar/contacts/newsletter columns

Expand Down Expand Up @@ -3656,7 +3662,10 @@ for (const name of Object.keys(obj)) {
//
// only run puppeteer if we're on the web server
//
if (env.NODE_ENV !== 'production' || env.WEB_HOST === os.hostname()) {
if (
env.NODE_ENV !== 'test' &&
(env.NODE_ENV !== 'production' || env.WEB_HOST === os.hostname())
) {
(async () => {
let browser;
try {
Expand Down Expand Up @@ -3713,6 +3722,10 @@ if (env.NODE_ENV !== 'production' || env.WEB_HOST === os.hostname()) {
err.isCodeBug = true;
logger.fatal(err);
}

// <https://stackoverflow.com/a/76505750>
// eslint-disable-next-line no-await-in-loop
await page.close();
}

// unlimited_domains, unlimited_aliases, smtp, imap, pop3, api, openpgp, wkd (boolean | string)
Expand Down Expand Up @@ -3742,11 +3755,22 @@ if (env.NODE_ENV !== 'production' || env.WEB_HOST === os.hostname()) {
logger.fatal(err);
}

if (browser)
browser
.close()
.then()
.catch((err) => logger.fatal(err));
if (browser) {
try {
// <https://stackoverflow.com/a/76505750>

const pages = await browser.pages();
for (const page of pages) {
// eslint-disable-next-line no-await-in-loop
await page.close();
}

await browser.close();
} catch (err) {
err.isCodeBug = true;
logger.fatal(err);
}
}
})();
}

Expand Down
Loading

0 comments on commit 8083077

Please sign in to comment.