-
Notifications
You must be signed in to change notification settings - Fork 224
BugError added for unexpected response from BulkOps API #6666
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
dbdabee to
d1afb9f
Compare
Coverage report
Test suite run success3435 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.') |
There was a problem hiding this comment.
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?
d1afb9f to
e4b116f
Compare
38bd5b0 to
b4e7710
Compare
e4b116f to
388de23
Compare
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/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
|

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
bulkOperationResponsewith two parts:bulkOperationanduserErrors. When the code gets thebulkOperationResponseback, the code looks to see ifuserErrorsexist before even looking at thebulkOperationpart.I wanted to deal with the case where the
bulkOperationvalue wasnull. However, if thebulkOperationvalue isnull, there should be something inuserErrors, and the code should have returned the error already (since it checksuserErrorsbeforebulkOperation). So, this case should never happen (this case being wherebulkOperationis null anduserErrorsis empty).I still wanted to write a case for this. So, I wrote
BugErrorthat alerts the team if this case ever happens. ABugErroris something like Sentry in Core.Testing
Added unit test to test this code path