Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace lodash methods #2454

Merged
merged 10 commits into from
Mar 12, 2024
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
"dependencies": {
"cron-parser": "^4.6.0",
"ioredis": "^5.3.2",
"lodash": "^4.17.21",
"msgpackr": "^1.10.1",
"node-abort-controller": "^3.1.1",
"semver": "^7.5.4",
Expand Down
5 changes: 2 additions & 3 deletions src/classes/flow-producer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { EventEmitter } from 'events';
import { get } from 'lodash';
import { Redis, ChainableCommander } from 'ioredis';
import { v4 } from 'uuid';
import {
Expand Down Expand Up @@ -279,15 +278,15 @@ export class FlowProducer extends EventEmitter {
const queue = this.queueFromNode(node, new QueueKeys(prefix), prefix);
const queueOpts = queuesOpts && queuesOpts[node.queueName];

const jobsOpts = get(queueOpts, 'defaultJobOptions');
const jobsOpts = queueOpts?.defaultJobOptions ?? {};
const jobId = node.opts?.jobId || v4();

const job = new this.Job(
queue,
node.name,
node.data,
{
...(jobsOpts ? jobsOpts : {}),
...jobsOpts,
...node.opts,
parent: parent?.parentOpts,
},
Expand Down
8 changes: 4 additions & 4 deletions src/classes/main-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
* Wrapper for sandboxing.
*
*/
import { toString } from 'lodash';
import { ChildProcessor } from './child-processor';
import { ParentCommand, ChildCommand } from '../enums';
import { errorToJSON } from '../utils';
import { errorToJSON, toString } from '../utils';

export default (
send: (msg: any) => Promise<void>,
Expand Down Expand Up @@ -33,10 +32,11 @@ export default (
process.on('SIGTERM', () => childProcessor.waitForCurrentJobAndExit());
process.on('SIGINT', () => childProcessor.waitForCurrentJobAndExit());

process.on('uncaughtException', async (err: Error) => {
if (!err.message) {
process.on('uncaughtException', async (err: any) => {
if (typeof err !== 'object') {
err = new Error(toString(err));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me a bit nervous, not sure if this is the correct thing to do. So if the error does not include .message we assume it is not an error object, but more likely just a string, and thus we create a new error here, which will create also an error stack. If we do not do this, then we call errorToJSON with a string , will this yield this a correct "value"? This uncaughException is emitted in userland code, out of our control... can we track who did add this code to see if we can understand the original intend?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that this change was added last year with worker threads support 0820985#diff-dba50fb04784a53f9826c6cb96929de2d71329d82f1d97418a13fc54a955f8d5R37-R39

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that in order not to create a regression (even though no test is testing this feature), we should implement our own toString then if we want to get rid of lodash.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, you did already 👍

}

await send({
cmd: ParentCommand.Failed,
value: errorToJSON(err),
Expand Down
5 changes: 2 additions & 3 deletions src/classes/queue.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { get } from 'lodash';
import { v4 } from 'uuid';
import {
BaseJobOptions,
Expand Down Expand Up @@ -112,15 +111,15 @@ export class Queue<
Connection,
);

this.jobsOpts = get(opts, 'defaultJobOptions') ?? {};
this.jobsOpts = opts?.defaultJobOptions ?? {};

this.waitUntilReady()
.then(client => {
if (!this.closing) {
client.hset(
this.keys.meta,
'opts.maxLenEvents',
get(opts, 'streams.events.maxLen', 10000),
opts?.streams?.events?.maxLen ?? 10000,
);
}
})
Expand Down
24 changes: 24 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,28 @@ export const errorToJSON = (value: any): Record<string, any> => {
return error;
};

const INFINITY = 1 / 0;

export const toString = (value: any): string => {
if (value == null) {
return '';
}
// Exit early for strings to avoid a performance hit in some environments.
if (typeof value === 'string') {
return value;
}
if (Array.isArray(value)) {
// Recursively convert values (susceptible to call stack limits).
return `${value.map(other => (other == null ? other : toString(other)))}`;
}
if (
typeof value == 'symbol' ||
Object.prototype.toString.call(value) == '[object Symbol]'
) {
return value.toString();
}
const result = `${value}`;
return result === '0' && 1 / value === -INFINITY ? '-0' : result;
};

export const QUEUE_EVENT_SUFFIX = ':qe';
Loading