Skip to content

ref: Remove some unnecessary conditions #14698

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

Merged
merged 4 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/lib/getTestMatrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function addIncludesForTestApp(
}

function getSentryDependencies(appName: string): string[] {
const packageJson = getPackageJson(appName) || {};
const packageJson = getPackageJson(appName);

const dependencies = {
...packageJson.devDependencies,
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('init', () => {
expect(initAndBindSpy).toHaveBeenCalledTimes(1);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
expect(optionsPassed?.integrations?.length).toBeGreaterThan(0);
expect(optionsPassed?.integrations.length).toBeGreaterThan(0);
});

test("doesn't install default integrations if told not to", () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/utils-hoist/vercelWaitUntil.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { GLOBAL_OBJ } from './worldwide';

interface VercelRequestContextGlobal {
get?(): {
waitUntil?: (task: Promise<unknown>) => void;
};
get?():
| {
waitUntil?: (task: Promise<unknown>) => void;
}
| undefined;
}

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,22 @@ function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (cleanedExtra && Object.keys(cleanedExtra).length) {
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (cleanedTags && Object.keys(cleanedTags).length) {
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (cleanedUser && Object.keys(cleanedUser).length) {
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (cleanedContexts && Object.keys(cleanedContexts).length) {
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
}

Expand Down Expand Up @@ -190,7 +190,7 @@ function applyFingerprintToEvent(event: Event, fingerprint: ScopeData['fingerpri
}

// If we have no data at all, remove empty array default
if (event.fingerprint && !event.fingerprint.length) {
if (!event.fingerprint.length) {
delete event.fingerprint;
}
}
2 changes: 1 addition & 1 deletion packages/core/src/utils/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export function merge<T>(initialObj: T, mergeObj: T, levels = 2): T {
}

// If the merge object is an empty object, and the initial object is not undefined, we return the initial object
if (initialObj && mergeObj && Object.keys(mergeObj).length === 0) {
if (initialObj && Object.keys(mergeObj).length === 0) {
return initialObj;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void {
event!.exception!.values!.forEach(exception => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
exception.stacktrace!.frames!.forEach(frame => {
if (filenameDebugIdMap && frame.filename) {
if (frame.filename) {
Copy link
Member Author

Choose a reason for hiding this comment

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

all of this is try-catched anyhow.

frame.debug_id = filenameDebugIdMap[frame.filename];
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export function spanToJSON(span: Span): Partial<SpanJSON> {
}

function spanIsOpenTelemetrySdkTraceBaseSpan(span: Span): span is OpenTelemetrySdkTraceBaseSpan {
const castSpan = span as OpenTelemetrySdkTraceBaseSpan;
const castSpan = span as Partial<OpenTelemetrySdkTraceBaseSpan>;
return !!castSpan.attributes && !!castSpan.startTime && !!castSpan.name && !!castSpan.endTime && !!castSpan.status;
}

Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('createSpanEnvelope', () => {

const spanEnvelope = createSpanEnvelope([span]);

const spanItem = spanEnvelope[1]?.[0]?.[1];
const spanItem = spanEnvelope[1][0]?.[1];
expect(spanItem).toEqual({
data: {
'sentry.origin': 'manual',
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('createSpanEnvelope', () => {

expect(beforeSendSpan).toHaveBeenCalled();

const spanItem = spanEnvelope[1]?.[0]?.[1];
const spanItem = spanEnvelope[1][0]?.[1];
expect(spanItem).toEqual({
data: {
'sentry.origin': 'manual',
Expand Down Expand Up @@ -242,7 +242,7 @@ describe('createSpanEnvelope', () => {

expect(beforeSendSpan).toHaveBeenCalled();

const spanItem = spanEnvelope[1]?.[0]?.[1];
const spanItem = spanEnvelope[1][0]?.[1];
expect(spanItem).toEqual({
data: {
'sentry.origin': 'manual',
Expand Down
6 changes: 1 addition & 5 deletions packages/core/test/lib/integrations/zoderrrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ class ZodError extends Error {
super();

const actualProto = new.target.prototype;
if (Object.setPrototypeOf) {
Object.setPrototypeOf(this, actualProto);
} else {
(this as any).__proto__ = actualProto;
}
Object.setPrototypeOf(this, actualProto);

this.name = 'ZodError';
this.issues = issues;
Expand Down
4 changes: 1 addition & 3 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ describe('startSpan', () => {
try {
await startSpan({ name: 'GET users/[id]' }, () => {
return startSpan({ name: 'SELECT * from users' }, childSpan => {
if (childSpan) {
childSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'db.query');
}
childSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'db.query');
return callback();
});
});
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/lib/transports/multiplexed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('makeMultiplexedTransport', () => {
const makeTransport = makeMultiplexedTransport(
createTestTransport((url, _, env) => {
expect(url).toBe(DSN2_URL);
expect(env[0]?.dsn).toBe(DSN2);
expect(env[0].dsn).toBe(DSN2);
}),
() => [DSN2],
);
Expand All @@ -134,7 +134,7 @@ describe('makeMultiplexedTransport', () => {
createTestTransport((url, release, env) => {
expect(url).toBe(DSN2_URL);
expect(release).toBe('[email protected]');
expect(env[0]?.dsn).toBe(DSN2);
expect(env[0].dsn).toBe(DSN2);
}),
() => [{ dsn: DSN2, release: '[email protected]' }],
);
Expand All @@ -150,7 +150,7 @@ describe('makeMultiplexedTransport', () => {
createTestTransport((url, release, env) => {
expect(url).toBe('http://google.com');
expect(release).toBe('[email protected]');
expect(env[0]?.dsn).toBe(DSN2);
expect(env[0].dsn).toBe(DSN2);
}),
() => [{ dsn: DSN2, release: '[email protected]' }],
);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/lib/transports/offline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ describe('makeOfflineTransport', () => {
// When it gets shifted out of the store, the sent_at header should be updated
const envelopes = getSentEnvelopes().map(parseEnvelope) as EventEnvelope[];
expect(envelopes[0]?.[0]).toBeDefined();
const sent_at = new Date(envelopes[0]![0]?.sent_at);
const sent_at = new Date(envelopes[0]![0].sent_at);

expect(sent_at.getTime()).toBeGreaterThan(testStartTime.getTime());
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/utils-hoist/envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('envelope', () => {
measurements: { inp: { value: expect.any(Number), unit: expect.any(String) } },
};

expect(spanEnvelopeItem[0]?.type).toBe('span');
expect(spanEnvelopeItem[0].type).toBe('span');
expect(spanEnvelopeItem[1]).toMatchObject(expectedObj);
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/utils-hoist/misc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe('addExceptionMechanism', () => {

addExceptionMechanism(event);

expect(event.exception.values[0]?.mechanism).toEqual(defaultMechanism);
expect(event.exception.values[0].mechanism).toEqual(defaultMechanism);
});

it('prefers current values to defaults', () => {
Expand All @@ -226,7 +226,7 @@ describe('addExceptionMechanism', () => {

addExceptionMechanism(event);

expect(event.exception.values[0]?.mechanism).toEqual(nonDefaultMechanism);
expect(event.exception.values[0].mechanism).toEqual(nonDefaultMechanism);
});

it('prefers incoming values to current values', () => {
Expand All @@ -239,7 +239,7 @@ describe('addExceptionMechanism', () => {
addExceptionMechanism(event, newMechanism);

// the new `handled` value took precedence
expect(event.exception.values[0]?.mechanism).toEqual({ type: 'instrument', handled: true, synthetic: true });
expect(event.exception.values[0].mechanism).toEqual({ type: 'instrument', handled: true, synthetic: true });
});

it('merges data values', () => {
Expand All @@ -251,7 +251,7 @@ describe('addExceptionMechanism', () => {

addExceptionMechanism(event, newMechanism);

expect(event.exception.values[0]?.mechanism.data).toEqual({
expect(event.exception.values[0].mechanism.data).toEqual({
function: 'addEventListener',
handler: 'organizeShoes',
target: 'closet',
Expand Down
14 changes: 7 additions & 7 deletions packages/feedback/src/core/components/Actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('Actor', () => {
const actorComponent = feedback!.createWidget();

expect(actorComponent.el).toBeInstanceOf(HTMLButtonElement);
expect(actorComponent.el?.textContent).toBe(TRIGGER_LABEL);
expect(actorComponent.el.textContent).toBe(TRIGGER_LABEL);
});

it('renders the correct aria label for the button', () => {
Expand All @@ -43,19 +43,19 @@ describe('Actor', () => {
// aria label is the same as trigger label when the trigger label isn't empty
const actorDefault = feedback!.createWidget({ triggerLabel: 'Button' });

expect(actorDefault.el?.textContent).toBe('Button');
expect(actorDefault.el?.ariaLabel).toBe('Button');
expect(actorDefault.el.textContent).toBe('Button');
expect(actorDefault.el.ariaLabel).toBe('Button');

// aria label is default text when trigger label is empty and aria isn't configured
const actorIcon = feedback!.createWidget({ triggerLabel: '' });

expect(actorIcon.el?.textContent).toBe('');
expect(actorIcon.el?.ariaLabel).toBe(TRIGGER_LABEL);
expect(actorIcon.el.textContent).toBe('');
expect(actorIcon.el.ariaLabel).toBe(TRIGGER_LABEL);

// aria label is the triggerAriaLabel if it's configured
const actorAria = feedback!.createWidget({ triggerLabel: 'Button', triggerAriaLabel: 'Aria' });

expect(actorAria.el?.textContent).toBe('Button');
expect(actorAria.el?.ariaLabel).toBe('Aria');
expect(actorAria.el.textContent).toBe('Button');
expect(actorAria.el.ariaLabel).toBe('Aria');
});
});
19 changes: 11 additions & 8 deletions packages/node/src/cron/node-cron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export interface NodeCronOptions {
}

export interface NodeCron {
schedule: (cronExpression: string, callback: () => void, options: NodeCronOptions) => unknown;
schedule: (cronExpression: string, callback: () => void, options: NodeCronOptions | undefined) => unknown;
}

/**
Expand All @@ -30,20 +30,23 @@ export interface NodeCron {
*/
export function instrumentNodeCron<T>(lib: Partial<NodeCron> & T): T {
return new Proxy(lib, {
get(target, prop: keyof NodeCron) {
get(target, prop) {
if (prop === 'schedule' && target.schedule) {
// When 'get' is called for schedule, return a proxied version of the schedule function
return new Proxy(target.schedule, {
apply(target, thisArg, argArray: Parameters<NodeCron['schedule']>) {
const [expression, callback, options] = argArray;

if (!options?.name) {
const name = options?.name;
const timezone = options?.timezone;

if (!name) {
throw new Error('Missing "name" for scheduled job. A name is required for Sentry check-in monitoring.');
}

async function monitoredCallback(): Promise<void> {
const monitoredCallback = async (): Promise<void> => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@AbhiPrasad (I think you worked on this initially) is it important that this is not an arrow function? Without this, it complains about stuff being mutateable as this is hoisted, I guess.

Copy link
Member

Choose a reason for hiding this comment

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

arrow function should be fine, it shouldn't rely on the this context.

return withMonitor(
options.name,
name,
async () => {
// We have to manually catch here and capture the exception because node-cron swallows errors
// https://github.com/node-cron/node-cron/issues/399
Expand All @@ -56,16 +59,16 @@ export function instrumentNodeCron<T>(lib: Partial<NodeCron> & T): T {
},
{
schedule: { type: 'crontab', value: replaceCronNames(expression) },
timezone: options?.timezone,
timezone,
},
);
}
};

return target.apply(thisArg, [expression, monitoredCallback, options]);
},
});
} else {
return target[prop];
return target[prop as keyof T];
}
},
});
Expand Down
10 changes: 5 additions & 5 deletions packages/node/src/integrations/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,18 @@ export const nodeContextIntegration = defineIntegration(_nodeContextIntegration)
function _updateContext(contexts: Contexts): Contexts {
// Only update properties if they exist

if (contexts?.app?.app_memory) {
if (contexts.app?.app_memory) {
contexts.app.app_memory = process.memoryUsage().rss;
}

if (contexts?.app?.free_memory && typeof (process as ProcessWithCurrentValues).availableMemory === 'function') {
if (contexts.app?.free_memory && typeof (process as ProcessWithCurrentValues).availableMemory === 'function') {
const freeMemory = (process as ProcessWithCurrentValues).availableMemory?.();
if (freeMemory != null) {
contexts.app.free_memory = freeMemory;
}
}

if (contexts?.device?.free_memory) {
if (contexts.device?.free_memory) {
contexts.device.free_memory = os.freemem();
}

Expand Down Expand Up @@ -226,7 +226,7 @@ export function getDeviceContext(deviceOpt: DeviceContextOptions | true): Device
// Sometimes os.uptime() throws due to lacking permissions: https://github.com/getsentry/sentry-javascript/issues/8202
let uptime;
try {
uptime = os.uptime && os.uptime();
uptime = os.uptime();
} catch (e) {
// noop
}
Expand All @@ -246,7 +246,7 @@ export function getDeviceContext(deviceOpt: DeviceContextOptions | true): Device
}

if (deviceOpt === true || deviceOpt.cpu) {
const cpuInfo: os.CpuInfo[] | undefined = os.cpus();
const cpuInfo = os.cpus() as os.CpuInfo[] | undefined;
const firstCpu = cpuInfo && cpuInfo[0];
if (firstCpu) {
device.processor_count = cpuInfo.length;
Expand Down
Loading
Loading