Skip to content

Conversation

@ericlee878
Copy link
Contributor

@ericlee878 ericlee878 commented Nov 25, 2025

Resolves: https://github.com/orgs/shop/projects/208/views/34?pane=issue&itemId=139446745&issue=shop%7Cissues-api-foundations%7C1113

Inspiration from: https://app.graphite.com/github/pr/Shopify/cli/6588/Initial-Setup-Bulk-Operations-Infrastructure-for-Shopify-CLI#comment-PRRC_kwDOHibhHs6WnG-H

Background
When the CLI calls the BulkOps API, the API returns a bulkOperationResponse with two parts: bulkOperation and userErrors. When the code gets the bulkOperationResponse back, the code looks to see if userErrors exist before even looking at the bulkOperation part.

I wanted to deal with the case where the bulkOperation value was null. However, if the bulkOperation value is null, there should be something in userErrors, and the code should have returned the error already (since it checks userErrors before bulkOperation). So, this case should never happen (this case being where bulkOperation is null and userErrors is empty).

I still wanted to write a case for this. So, I wrote BugError that alerts the team if this case ever happens. A BugError is something like Sentry in Core.

Testing
Added unit test to test this code path

@ericlee878 ericlee878 marked this pull request as ready for review November 25, 2025 21:47
@ericlee878 ericlee878 requested a review from a team as a code owner November 25, 2025 21:47
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@ericlee878 ericlee878 requested review from jordanverasamy and teddyhwang and removed request for jordanverasamy November 25, 2025 21:49
@ericlee878 ericlee878 force-pushed the 11-24-variable-validation-for-BulkOps-CLI branch from dbdabee to d1afb9f Compare November 25, 2025 21:51
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
79.22% (+0% 🔼)
13793/17410
🟡 Branches
73.15% (+0.01% 🔼)
6733/9205
🟡 Functions 79.32% 3541/4464
🟡 Lines
79.6% (+0% 🔼)
13034/16375

Test suite run success

3435 tests passing in 1393 suites.

Report generated by 🧪jest coverage report action from 388de23

headline: 'Bulk operation not created succesfully.',
body: 'This is an unexpected error. Please try again later.',
})
throw new BugError('Bulk operation response returned null with no error message.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly sure if BugError is the right way to throw alerts here. @nickwesselman Is there a way CLI convention for this?

@jordanverasamy jordanverasamy changed the base branch from 11-24-Stdin-for-query-input-BulkOps-CLI to graphite-base/6666 November 27, 2025 03:45
@ericlee878 ericlee878 force-pushed the 11-24-variable-validation-for-BulkOps-CLI branch from d1afb9f to e4b116f Compare December 1, 2025 21:59
@ericlee878 ericlee878 changed the base branch from graphite-base/6666 to 11-24-Stdin-for-query-input-BulkOps-CLI December 1, 2025 21:59
@ericlee878 ericlee878 force-pushed the 11-24-variable-validation-for-BulkOps-CLI branch from e4b116f to 388de23 Compare December 1, 2025 22:03
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/system.d.ts
@@ -62,4 +62,23 @@ export declare function isCI(): boolean;
  *
  * @returns True if the current environment is a WSL environment.
  */
-export declare function isWsl(): Promise<boolean>;
\ No newline at end of file
+export declare function isWsl(): Promise<boolean>;
+/**
+ * Check if stdin has piped data available.
+ * This distinguishes between actual piped input (e.g., )
+ * and non-TTY environments without input (e.g., CI).
+ *
+ * @returns True if stdin is receiving piped data or file redirect, false otherwise.
+ */
+export declare function isStdinPiped(): boolean;
+/**
+ * Reads all data from stdin and returns it as a string.
+ * This is useful for commands that accept input via piping.
+ *
+ * @example
+ * // Usage: echo "your query" | shopify app execute
+ * const query = await readStdin()
+ *
+ * @returns A promise that resolves with the stdin content, or undefined if stdin is a TTY.
+ */
+export declare function readStdinString(): Promise<string | undefined>;
\ No newline at end of file

Base automatically changed from 11-24-Stdin-for-query-input-BulkOps-CLI to main December 1, 2025 22:18
@ericlee878 ericlee878 added this pull request to the merge queue Dec 2, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 2, 2025
@ericlee878 ericlee878 added this pull request to the merge queue Dec 2, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 2, 2025
@ericlee878 ericlee878 added this pull request to the merge queue Dec 2, 2025
Merged via the queue into main with commit 328e987 Dec 2, 2025
23 checks passed
@ericlee878 ericlee878 deleted the 11-24-variable-validation-for-BulkOps-CLI branch December 2, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants