Skip to content

Commit

Permalink
fix: switching to use spawn instead of exec
Browse files Browse the repository at this point in the history
  • Loading branch information
jharvey10 committed Sep 12, 2023
1 parent fb0dd74 commit 71654c7
Show file tree
Hide file tree
Showing 13 changed files with 164 additions and 58 deletions.
34 changes: 0 additions & 34 deletions src/main/core/exec.ts

This file was deleted.

6 changes: 4 additions & 2 deletions src/main/core/get-project-root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/
import { exec } from './exec.js'
import { runCommand } from './run-command.js'

/**
* Finds and returns the root-most directory of the analyzed project's source tree.
Expand All @@ -14,5 +14,7 @@ import { exec } from './exec.js'
* @returns The path of the analyzed project's root directory or null.
*/
export async function getProjectRoot(cwd: string): Promise<string> {
return await exec('git rev-parse --show-toplevel', { cwd })
const result = await runCommand('git rev-parse --show-toplevel', { cwd })

return result.stdout
}
84 changes: 84 additions & 0 deletions src/main/core/run-command.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright IBM Corp. 2023, 2023
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/
import childProcess from 'node:child_process'

import { RunCommandError } from '../exceptions/run-command-error.js'
import { guardShell } from './guard-shell.js'

interface Result {
stdout: string
stderr: string
exitCode: number
}

/**
* Runs a command using childProcess. Throws an exception if the command fails or exits with a
* non-zero exit code.
*
* @param cmd - The command to invoke.
* @param options - Options to include along with the command.
* @param rejectOnError - Whether or not to reject the resulting promise when a non-zero exit code
* is encountered.
* @returns The standard output from the command.
*/
export async function runCommand(
cmd: string,
options: childProcess.SpawnOptions = {},
rejectOnError: boolean = true
) {
guardShell(cmd)

const execOptions = {
env: process.env,
shell: true,
...options
}

return await new Promise<Result>((resolve, reject) => {
let outData = ''
let errorData = ''

const proc = childProcess.spawn(cmd, execOptions)

proc.stdout?.on('data', (data) => {
outData += data.toString()
})

proc.stderr?.on('data', (data) => {
errorData += data.toString()
})

proc.on('error', (err) => {
if (rejectOnError) {
reject(err)
} else {
resolve({
exitCode: 'errno' in err && typeof err.errno === 'number' ? err.errno : -1,
stderr: errorData,
stdout: outData
})
}
})

proc.on('close', (exitCode) => {
if (exitCode !== 0 && rejectOnError) {
reject(
new RunCommandError({
exitCode: exitCode ?? 999,
stderr: errorData.trim(),
stdout: outData.trim()
})
)
}
resolve({
exitCode: exitCode ?? 999,
stderr: errorData.trim(),
stdout: outData.trim()
})
})
})
}
2 changes: 1 addition & 1 deletion src/main/exceptions/invalid-root-path-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* Exception thrown when protecting exec commands from forbidden characters.
*/
export class InvalidRootPathError extends Error {
constructor(cwd: string, root: string) {
constructor(root: string, cwd: string) {
super(`${cwd} is not a subpath of ${root}`)
}
}
21 changes: 21 additions & 0 deletions src/main/exceptions/run-command-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright IBM Corp. 2023, 2023
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

interface RunCommandErrorReason {
exitCode: number
stderr: string
stdout: string
}

/**
* Error indicating a non-zero exit code of a command that was run via `runCommand`.
*/
export class RunCommandError extends Error {
constructor(reason: RunCommandErrorReason) {
super(JSON.stringify(reason))
}
}
9 changes: 5 additions & 4 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
* LICENSE file in the root directory of this source tree.
*/

import { exec } from './core/exec.js'
import { initializeOpenTelemetry } from './core/initialize-open-telemetry.js'
import { createLogFilePath } from './core/log/create-log-file-path.js'
import { Logger } from './core/log/logger.js'
import * as ResourceAttributes from './core/resource-attributes.js'
import { runCommand } from './core/run-command.js'
import { tokenizeRepository } from './core/tokenize-repository.js'
import { getTelemetryPackageData } from './scopes/npm/get-telemetry-package-data.js'

/*
1. read command line arguments
Expand Down Expand Up @@ -47,14 +48,14 @@ async function run() {

// TODO: handle non-existant remote
// TODO: move this logic elsewhere
const gitOrigin = await exec('git remote get-url origin')
const repository = tokenizeRepository(gitOrigin)
const gitOrigin = await runCommand('git remote get-url origin')
const repository = tokenizeRepository(gitOrigin.stdout)

const metricReader = initializeOpenTelemetry({
[ResourceAttributes.EMITTER_NAME]: telemetryName,
[ResourceAttributes.EMITTER_VERSION]: telemetryVersion,
[ResourceAttributes.PROJECT_ID]: config.projectId,
[ResourceAttributes.ANALYZED_RAW]: gitOrigin,
[ResourceAttributes.ANALYZED_RAW]: gitOrigin.stdout,
[ResourceAttributes.ANALYZED_HOST]: repository.host,
[ResourceAttributes.ANALYZED_OWNER]: repository.owner,
[ResourceAttributes.ANALYZED_REPOSITORY]: repository.repository,
Expand Down
11 changes: 7 additions & 4 deletions src/main/scopes/npm/find-installing-packages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import { exec } from '../../core/exec.js'
import { getProjectRoot } from '../../core/get-project-root.js'
import { runCommand } from '../../core/run-command.js'
import { findInstallersFromTree } from './find-installers-from-tree.js'
import { findScannableDirectories } from './find-scannable-directories.js'
import { type InstallingPackage } from './interfaces.js'
Expand All @@ -21,22 +20,26 @@ import { type InstallingPackage } from './interfaces.js'
* returned.
*
* @param cwd - Current working directory to use when finding installing packages.
* @param root - The root-most directory to consider when searching for installers.
* @param packageName - The name of the package to search for.
* @param packageVersion - The exact version of the package to search for.
* @returns A possibly empty array of installing packages.
*/
export async function findInstallingPackages(
cwd: string,
root: string,
packageName: string,
packageVersion: string
): Promise<InstallingPackage[]> {
const root = await getProjectRoot(cwd)
const dirs = await findScannableDirectories(cwd, root)

let installers: InstallingPackage[] = []

for (const d of dirs) {
const dependencyTree = JSON.parse(await exec('npm ls --all --json', { cwd: d }))
// Allow this command to try and obtain results even if it exited with a total or partial error
const result = await runCommand('npm ls --all --json', { cwd: d }, false)

const dependencyTree = JSON.parse(result.stdout)

installers = findInstallersFromTree(dependencyTree, packageName, packageVersion)

Expand Down
2 changes: 1 addition & 1 deletion src/main/scopes/npm/find-scannable-directories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function findScannableDirectories(cwd: string, root: string) {

// (if cwd is not a subpath of root, throw an exception)
if (path.relative(root, cwd).startsWith('..')) {
throw new InvalidRootPathError(cwd, root)
throw new InvalidRootPathError(root, cwd)
}

do {
Expand Down
5 changes: 3 additions & 2 deletions src/main/scopes/npm/get-package-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/
import { exec } from '../../core/exec.js'
import { runCommand } from '../../core/run-command.js'
import { type PackageData } from './interfaces.js'

/**
Expand All @@ -16,5 +16,6 @@ import { type PackageData } from './interfaces.js'
* @returns An object containing details about the package.
*/
export async function getPackageData(packagePath: string): Promise<PackageData> {
return JSON.parse(await exec('npm pkg get name version', { cwd: packagePath }))
const result = await runCommand('npm pkg get name version', { cwd: packagePath })
return JSON.parse(result.stdout)
}
8 changes: 7 additions & 1 deletion src/main/scopes/npm/npm-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/
import { type Logger } from '../../core/log/logger.js'
import { Trace } from '../../core/log/trace.js'
import { Scope } from '../../core/scope.js'
import { findInstallingPackages } from './find-installing-packages.js'
import { getPackageData } from './get-package-data.js'
Expand All @@ -15,28 +16,33 @@ import { DependencyMetric } from './metrics/dependency-metric.js'
*/
export class NpmScope extends Scope {
private readonly cwd: string
private readonly root: string
protected override logger: Logger
public name = 'npm'

/**
* Constructs an NpmScope.
*
* @param cwd - The directory representing the instrumented package.
* @param root - The root-most directory to consider for dependency information.
* @param logger - Injected logger dependency.
*/
public constructor(cwd: string, logger: Logger) {
public constructor(cwd: string, root: string, logger: Logger) {
super()
this.cwd = cwd
this.root = root
this.logger = logger
}

@Trace()
public override async run(): Promise<void> {
const { name: instrumentedPkgName, version: instrumentedPkgVersion } = await getPackageData(
this.cwd
)

const installingPackages = await findInstallingPackages(
this.cwd,
this.root,
instrumentedPkgName,
instrumentedPkgVersion
)
Expand Down
22 changes: 19 additions & 3 deletions src/test/scopes/npm/find-installing-packages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* LICENSE file in the root directory of this source tree.
*/

import path from 'path'
import { describe, expect, it } from 'vitest'

import { findInstallingPackages } from '../../../main/scopes/npm/find-installing-packages.js'
Expand All @@ -19,21 +20,36 @@ import { Fixture } from '../../__utils/fixture.js'
describe('findInstallingPackages', () => {
it('correctly finds installing package data', async () => {
const fixture = new Fixture('projects/basic-project/node_modules/instrumented')
const pkgs = await findInstallingPackages(fixture.path, 'instrumented', '0.1.0')
const pkgs = await findInstallingPackages(
fixture.path,
path.join(fixture.path, '..', '..'),
'instrumented',
'0.1.0'
)

expect(pkgs).toMatchSnapshot()
})

it('finds no results for an unknown package', async () => {
const fixture = new Fixture('projects/basic-project/node_modules/instrumented')
const pkgs = await findInstallingPackages(fixture.path, 'not-here', '0.1.0')
const pkgs = await findInstallingPackages(
fixture.path,
path.join(fixture.path, '..', '..'),
'not-here',
'0.1.0'
)

expect(pkgs).toMatchSnapshot()
})

it('finds no results for an known package at an unknown version', async () => {
const fixture = new Fixture('projects/basic-project/node_modules/instrumented')
const pkgs = await findInstallingPackages(fixture.path, 'instrumented', '0.3.0')
const pkgs = await findInstallingPackages(
fixture.path,
path.join(fixture.path, '..', '..'),
'instrumented',
'0.3.0'
)

expect(pkgs).toMatchSnapshot()
})
Expand Down
14 changes: 9 additions & 5 deletions src/test/scopes/npm/get-telemetry-package-data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@
*/
import { describe, expect, it, vi } from 'vitest'

import * as exec from '../../../main/core/exec.js'
import * as exec from '../../../main/core/run-command.js'
import * as getPackageData from '../../../main/scopes/npm/get-package-data.js'
import { getTelemetryPackageData } from '../../../main/scopes/npm/get-telemetry-package-data.js'

const spy = vi.spyOn(getPackageData, 'getPackageData')

vi.spyOn(exec, 'exec').mockResolvedValue(JSON.stringify({
name: 'test-1',
version: '1.0.0'
}))
vi.spyOn(exec, 'runCommand').mockResolvedValue({
exitCode: 0,
stdout: JSON.stringify({
name: 'test-1',
version: '1.0.0'
}),
stderr: ''
})

describe('getTelemetryPackageData', () => {
it('correctly reads name and version', async () => {
Expand Down
Loading

0 comments on commit 71654c7

Please sign in to comment.