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

Print logs to stderr #4922

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
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
8 changes: 8 additions & 0 deletions .changeset/silent-years-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@shopify/ui-extensions-dev-console-app': patch
'@shopify/cli-kit': patch
'@shopify/theme': patch
'@shopify/app': patch
---

Print all log messages to stderr instead of stdout
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/env/pull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import AppCommand, {AppCommandOutput} from '../../../utilities/app-command.js'
import {linkedAppContext} from '../../../services/app-context.js'
import {Flags} from '@oclif/core'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {outputInfo} from '@shopify/cli-kit/node/output'
import {outputResult} from '@shopify/cli-kit/node/output'
import {joinPath} from '@shopify/cli-kit/node/path'

export default class EnvPull extends AppCommand {
Expand Down Expand Up @@ -37,7 +37,7 @@ export default class EnvPull extends AppCommand {
userProvidedConfigName: flags.config,
})
const envFile = joinPath(app.directory, flags['env-file'] ?? getDotEnvFileName(app.configuration.path))
outputInfo(await pullEnv({app, remoteApp, organization, envFile}))
outputResult(await pullEnv({app, remoteApp, organization, envFile}))
return {app}
}
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/env/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {linkedAppContext} from '../../../services/app-context.js'
import {showEnv} from '../../../services/app/env/show.js'
import AppCommand, {AppCommandOutput} from '../../../utilities/app-command.js'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {outputInfo} from '@shopify/cli-kit/node/output'
import {outputResult} from '@shopify/cli-kit/node/output'

export default class EnvShow extends AppCommand {
static summary = 'Display app and extensions environment variables.'
Expand All @@ -25,7 +25,7 @@ export default class EnvShow extends AppCommand {
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
})
outputInfo(await showEnv(app, remoteApp, organization))
outputResult(await showEnv(app, remoteApp, organization))
return {app}
}
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import AppCommand, {AppCommandOutput} from '../../utilities/app-command.js'
import {linkedAppContext} from '../../services/app-context.js'
import {Flags} from '@oclif/core'
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
import {outputInfo} from '@shopify/cli-kit/node/output'
import {outputResult} from '@shopify/cli-kit/node/output'
import {renderInfo} from '@shopify/cli-kit/node/ui'

export default class AppInfo extends AppCommand {
Expand Down Expand Up @@ -48,7 +48,7 @@ export default class AppInfo extends AppCommand {
developerPlatformClient,
})
if (typeof results === 'string' || 'value' in results) {
outputInfo(results)
outputResult(results)
} else {
renderInfo({customSections: results})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {renderJsonLogs} from './render-json-logs.js'
import {pollAppLogs} from './poll-app-logs.js'
import {handleFetchAppLogsError} from '../utils.js'
import {testDeveloperPlatformClient} from '../../../models/app/app.test-data.js'
import {outputInfo} from '@shopify/cli-kit/node/output'
import {outputInfo, outputResult} from '@shopify/cli-kit/node/output'
import {describe, expect, vi, test, beforeEach, afterEach} from 'vitest'
import {formatLocalDate} from '@shopify/cli-kit/common/string'

Expand Down Expand Up @@ -51,7 +51,7 @@ describe('renderJsonLogs', () => {
storeNameById,
})

expect(outputInfo).toHaveBeenNthCalledWith(
expect(outputResult).toHaveBeenNthCalledWith(
1,
JSON.stringify({
shopId: '1',
Expand All @@ -61,7 +61,7 @@ describe('renderJsonLogs', () => {
storeName: 'storeName',
}),
)
expect(outputInfo).toHaveBeenNthCalledWith(
expect(outputResult).toHaveBeenNthCalledWith(
2,
JSON.stringify({
shopId: '1',
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('renderJsonLogs', () => {
storeNameById,
})

expect(outputInfo).not.toHaveBeenCalled()
expect(outputResult).not.toHaveBeenCalled()
})

test('should handle error response and retry as expected', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
toFormattedAppLogJson,
parseAppLogPayload,
} from '../utils.js'
import {outputInfo} from '@shopify/cli-kit/node/output'
import {outputInfo, outputResult} from '@shopify/cli-kit/node/output'

export async function renderJsonLogs({
pollOptions: {cursor, filters, jwtToken},
Expand Down Expand Up @@ -53,7 +53,7 @@
return
}

outputInfo(
outputResult(
toFormattedAppLogJson({
appLog: log,
appLogPayload: parseAppLogPayload(log.payload, log.log_type),
Expand All @@ -69,7 +69,7 @@
options: {variables, developerPlatformClient},
pollOptions: {
jwtToken: nextJwtToken || jwtToken,
cursor: nextCursor || cursor,

Check warning on line 72 in packages/app/src/cli/services/app-logs/logs-command/render-json-logs.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/app-logs/logs-command/render-json-logs.ts#L72

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
filters,
},
storeNameById,
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/app-logs/sources.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {sourcesForApp} from './utils.js'
import {sources} from './sources.js'
import {testApp} from '../../models/app/app.test-data.js'
import {outputInfo, formatSection} from '@shopify/cli-kit/node/output'
import {outputResult, formatSection} from '@shopify/cli-kit/node/output'
import {describe, test, vi, expect} from 'vitest'

vi.mock('@shopify/cli-kit/node/output')
Expand All @@ -19,6 +19,6 @@ describe('sources', () => {
// Then
expect(formatSection).toHaveBeenCalledWith('extensions', 'extensions.source1\nextensions.source2')
expect(formatSection).toHaveBeenCalledWith('arbitrary', 'arbitrary.text')
expect(outputInfo).toHaveBeenCalledWith('formatted section')
expect(outputResult).toHaveBeenCalledWith('formatted section')
})
})
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/app-logs/sources.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {AppInterface} from '../../models/app/app.js'
import {sourcesForApp} from '../../services/app-logs/utils.js'
import {formatSection, outputInfo} from '@shopify/cli-kit/node/output'
import {formatSection, outputResult} from '@shopify/cli-kit/node/output'

export function sources(app: AppInterface) {
const sources = sourcesForApp(app)
Expand All @@ -22,6 +22,6 @@ export function sources(app: AppInterface) {
})

for (const [namespace, sources] of sourcesByNamespace) {
outputInfo(formatSection(namespace, sources.join('\n')))
outputResult(formatSection(namespace, sources.join('\n')))
}
}
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/generate-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('generateSchemaService', () => {
const stdout = true
const orgId = '123'
const mockOutput = vi.fn()
vi.spyOn(output, 'outputInfo').mockImplementation(mockOutput)
vi.spyOn(output, 'outputResult').mockImplementation(mockOutput)

// When
await generateSchemaService({
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/generate-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {ExtensionInstance} from '../models/extensions/extension-instance.js'
import {FunctionConfigType} from '../models/extensions/specifications/function.js'
import {AppLinkedInterface} from '../models/app/app.js'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputContent, outputInfo} from '@shopify/cli-kit/node/output'
import {outputContent, outputInfo, outputResult} from '@shopify/cli-kit/node/output'
import {writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'

Expand Down Expand Up @@ -43,7 +43,7 @@ export async function generateSchemaService(options: GenerateSchemaOptions) {
}))

if (stdout) {
outputInfo(definition)
outputResult(definition)
} else {
const outputPath = joinPath(extension.directory, 'schema.graphql')
await writeFile(outputPath, definition)
Expand Down
12 changes: 6 additions & 6 deletions packages/app/src/cli/services/logs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
testOrganizationStore,
} from '../models/app/app.test-data.js'
import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
import {consoleLog} from '@shopify/cli-kit/node/output'
import {outputInfo, outputResult} from '@shopify/cli-kit/node/output'
import {AbortError} from '@shopify/cli-kit/node/error'
import {describe, test, vi, expect, beforeEach} from 'vitest'
import {renderInfo} from '@shopify/cli-kit/node/ui'
Expand Down Expand Up @@ -54,7 +54,7 @@ describe('logs', () => {
})

// Then
expect(consoleLog).toHaveBeenCalledWith('{"message":"Waiting for app logs..."}')
expect(outputInfo).toHaveBeenCalledWith('{"message":"Waiting for app logs..."}')
expect(spy).toHaveBeenCalled()
})

Expand All @@ -78,7 +78,7 @@ describe('logs', () => {
})

// Then
expect(consoleLog).toHaveBeenCalledWith('Waiting for app logs...\n')
expect(outputInfo).toHaveBeenCalledWith('Waiting for app logs...\n')
expect(spy).toHaveBeenCalled()
})

Expand Down Expand Up @@ -156,8 +156,8 @@ describe('logs', () => {
const expectedStoreMap = new Map()
expectedStoreMap.set('1', 'store-fqdn')
expectedStoreMap.set('2', 'other-fqdn')
expect(consoleLog).toHaveBeenCalledWith('{"subscribedToStores":["store-fqdn","other-fqdn"]}')
expect(consoleLog).toHaveBeenCalledWith('{"message":"Waiting for app logs..."}')
expect(outputResult).toHaveBeenCalledWith('{"subscribedToStores":["store-fqdn","other-fqdn"]}')
expect(outputInfo).toHaveBeenCalledWith('{"message":"Waiting for app logs..."}')
expect(spy).toHaveBeenCalledWith({
options: {
developerPlatformClient: expect.anything(),
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('logs', () => {
const expectedStoreMap = new Map()
expectedStoreMap.set('1', 'store-fqdn')
expectedStoreMap.set('2', 'other-fqdn')
expect(consoleLog).toHaveBeenCalledWith('Waiting for app logs...\n')
expect(outputInfo).toHaveBeenCalledWith('Waiting for app logs...\n')
expect(spy).toHaveBeenCalledWith({
options: {
developerPlatformClient: expect.anything(),
Expand Down
8 changes: 4 additions & 4 deletions packages/app/src/cli/services/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {getAppConfigurationFileName} from '../models/app/loader.js'
import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
import {Organization, OrganizationApp, OrganizationStore} from '../models/organization.js'
import {AbortError} from '@shopify/cli-kit/node/error'
import {consoleLog} from '@shopify/cli-kit/node/output'
import {outputInfo, outputResult} from '@shopify/cli-kit/node/output'
import {renderInfo} from '@shopify/cli-kit/node/ui'
import {basename} from '@shopify/cli-kit/node/path'

Expand Down Expand Up @@ -66,8 +66,8 @@ export async function logs(commandOptions: LogsOptions) {
}

if (commandOptions.format === 'json') {
consoleLog(JSON.stringify({subscribedToStores: commandOptions.storeFqdns}))
consoleLog(JSON.stringify({message: 'Waiting for app logs...'}))
outputResult(JSON.stringify({subscribedToStores: commandOptions.storeFqdns}))
outputInfo(JSON.stringify({message: 'Waiting for app logs...'}))
await renderJsonLogs({
options: {
variables,
Expand All @@ -77,7 +77,7 @@ export async function logs(commandOptions: LogsOptions) {
storeNameById: logsConfig.storeNameById,
})
} else {
consoleLog('Waiting for app logs...\n')
outputInfo('Waiting for app logs...\n')
await renderLogs({
options: {
variables,
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/versions-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {AppLinkedInterface} from '../models/app/app.js'
import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
import {Organization, OrganizationApp} from '../models/organization.js'
import colors from '@shopify/cli-kit/node/colors'
import {outputContent, outputInfo, outputToken, unstyled} from '@shopify/cli-kit/node/output'
import {outputContent, outputInfo, outputResult, outputToken, unstyled} from '@shopify/cli-kit/node/output'
import {formatDate} from '@shopify/cli-kit/common/string'
import {AbortError} from '@shopify/cli-kit/node/error'
import {basename} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -94,7 +94,7 @@ export default async function versionList(options: VersionListOptions) {
const {appVersions, totalResults} = await fetchAppVersions(developerPlatformClient, remoteApp, options.json)

if (options.json) {
return outputInfo(JSON.stringify(appVersions, null, 2))
return outputResult(JSON.stringify(appVersions, null, 2))
}

renderCurrentlyUsedConfigInfo({
Expand Down
8 changes: 4 additions & 4 deletions packages/app/src/cli/services/webhook/trigger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
testOrganizationApp,
} from '../../models/app/app.test-data.js'
import {loadApp} from '../../models/app/loader.js'
import {outputSuccess, consoleError} from '@shopify/cli-kit/node/output'
import {outputSuccess, outputWarn} from '@shopify/cli-kit/node/output'
import {describe, expect, vi, test, beforeEach} from 'vitest'

const samplePayload = '{ "sampleField": "SampleValue" }'
Expand Down Expand Up @@ -83,7 +83,7 @@ describe('webhookTriggerService', () => {

// Then
expectCalls(aVersion, anOrganizationId)
expect(consoleError).toHaveBeenCalledWith(`Request errors:\n · Some error\n · Another error`)
expect(outputWarn).toHaveBeenCalledWith(`Request errors:\n · Some error\n · Another error`)
})

test('Safe notification in case of unexpected request errors', async () => {
Expand All @@ -106,7 +106,7 @@ describe('webhookTriggerService', () => {

// Then
expectCalls(aVersion, anOrganizationId)
expect(consoleError).toHaveBeenCalledWith(`Request errors:\n${JSON.stringify(response.userErrors)}`)
expect(outputWarn).toHaveBeenCalledWith(`Request errors:\n${JSON.stringify(response.userErrors)}`)
})

test('notifies about real delivery being sent', async () => {
Expand Down Expand Up @@ -240,7 +240,7 @@ describe('webhookTriggerService', () => {
anOrganizationId,
)
expect(triggerLocalWebhook).toHaveBeenCalledWith(aFullLocalAddress, samplePayload, sampleHeaders)
expect(consoleError).toHaveBeenCalledWith('Localhost delivery failed')
expect(outputWarn).toHaveBeenCalledWith('Localhost delivery failed')
})
})

Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/cli/services/webhook/trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {collectAddressAndMethod, collectApiVersion, collectCredentials, collectT
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppLinkedInterface} from '../../models/app/app.js'
import {OrganizationApp} from '../../models/organization.js'
import {consoleError, outputSuccess} from '@shopify/cli-kit/node/output'
import {outputWarn, outputSuccess} from '@shopify/cli-kit/node/output'

export interface WebhookTriggerInput {
app: AppLinkedInterface
Expand Down Expand Up @@ -76,7 +76,7 @@ async function sendSample(options: WebhookTriggerOptions) {
const sample = await getWebhookSample(options.developerPlatformClient, variables, options.organizationId)

if (!sample.success) {
consoleError(`Request errors:\n${formatErrors(sample.userErrors)}`)
outputWarn(`Request errors:\n${formatErrors(sample.userErrors)}`)
return
}

Expand All @@ -88,7 +88,7 @@ async function sendSample(options: WebhookTriggerOptions) {
return
}

consoleError('Localhost delivery failed')
outputWarn('Localhost delivery failed')
return
}

Expand Down
16 changes: 6 additions & 10 deletions packages/cli-kit/src/private/node/ui.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {collectLog, consoleLog, Logger, LogLevel, outputWhereAppropriate} from '../../public/node/output.js'
import {output, Logger, LogLevel} from '../../public/node/output.js'
import {isUnitTest} from '../../public/node/context/local.js'
import {treeKill} from '../../public/node/tree-kill.js'
import {ReactElement} from 'react'
Expand All @@ -11,20 +11,16 @@ interface RenderOnceOptions {
renderOptions?: RenderOptions
}

export function renderOnce(
element: JSX.Element,
{logLevel = 'info', logger = consoleLog, renderOptions}: RenderOnceOptions,
) {
const {output, unmount} = renderString(element, renderOptions)
export function renderOnce(element: JSX.Element, {logLevel = 'info', renderOptions}: RenderOnceOptions) {
const {output: renderedString, unmount} = renderString(element, renderOptions)

if (output) {
if (isUnitTest()) collectLog(logLevel, output)
outputWhereAppropriate(logLevel, logger, output)
if (renderedString) {
output(renderedString, logLevel)
}

unmount()

return output
return renderedString
}

export async function render(element: JSX.Element, options?: RenderOptions) {
Expand Down
11 changes: 2 additions & 9 deletions packages/cli-kit/src/private/node/ui/alert.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Alert, AlertProps} from './components/Alert.js'
import {renderOnce} from '../ui.js'
import {consoleError, consoleLog, consoleWarn, Logger, LogLevel} from '../../../public/node/output.js'
import {LogLevel} from '../../../public/node/output.js'
import React from 'react'
import {RenderOptions} from 'ink'

Expand All @@ -11,13 +11,6 @@ const typeToLogLevel: {[key in AlertProps['type']]: LogLevel} = {
error: 'error',
}

const typeToLogger: {[key in AlertProps['type']]: Logger} = {
info: consoleLog,
warning: consoleWarn,
success: consoleLog,
error: consoleError,
}

export interface AlertOptions extends AlertProps {
renderOptions?: RenderOptions
}
Expand Down Expand Up @@ -47,6 +40,6 @@ export function alert({
orderedNextSteps={orderedNextSteps}
customSections={customSections}
/>,
{logLevel: typeToLogLevel[type], logger: typeToLogger[type], renderOptions},
{logLevel: typeToLogLevel[type], renderOptions},
)
}
Loading
Loading