Skip to content

Fix unhelpful error when extension locale file has invalid UTF-8 (shop/issues-develop#21558)#7404

Merged
isaacroldan merged 2 commits into
mainfrom
04-27-fix_extension_locale_invalid_utf8_error
Apr 29, 2026
Merged

Fix unhelpful error when extension locale file has invalid UTF-8 (shop/issues-develop#21558)#7404
isaacroldan merged 2 commits into
mainfrom
04-27-fix_extension_locale_invalid_utf8_error

Conversation

@isaacroldan

@isaacroldan isaacroldan commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Fixes shop/issues-develop#21558

When an extension's locales/*.json file contains invalid UTF-8 (for example, a stray Latin-1 à left in an Italian translation), shopify app dev and shopify app deploy aborted with a generic INTERNAL_SERVER_ERROR raised server-side from String#strip in the framework's LocalizationUpdate#decode_dictionary. The error gave no hint about which locale file was wrong, leaving users to bisect their translations by hand.

WHAT is this pull request doing?

  • packages/app/src/cli/utilities/extensions/locales-configuration.ts: read each locale as a raw Buffer, run Buffer.isUtf8 (Node ≥ 18.14, well within the package's >=20.10.0 engine range) before base64-encoding, and throw an AbortError naming the offending file with an actionable hint to re-save it as UTF-8.
  • Added a regression test that writes a stray 0xE0 byte mirroring the example from the issue and asserts on the title and tryMessage.
  • Patch changeset for @shopify/app.

How to test your changes?

  1. In any extension that supports locales (e.g. checkout_ui_extension), write a locale file with invalid UTF-8:
    printf '{"hello":"sar\xE0"}' > extensions/<name>/locales/it.json
  2. Run shopify app deploy (or dev).
  3. Expect a CLI-side abort identifying locales/it.json and suggesting UTF-8 re-save — no INTERNAL_SERVER_ERROR.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

…p/issues-develop#21558)

Validate UTF-8 encoding of locale JSON files in `loadLocalesConfig`
before base64-encoding them for upload. Previously, a single invalid
byte (e.g. a Latin-1 "à" left in an Italian translation) would reach
the server and surface only as a generic INTERNAL_SERVER_ERROR raised
from `String#strip` deep in the framework's localization decoder. The
new check fails fast on the CLI side with the offending locale file
path and an actionable hint to re-save it as UTF-8.
Copilot AI review requested due to automatic review settings April 27, 2026 13:20
@isaacroldan isaacroldan requested review from a team as code owners April 27, 2026 13:20

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the UX around UI extension locale uploads by detecting invalid UTF-8 locale files locally and failing early with a clear, actionable error (instead of letting the server reject the upload with a generic internal error).

Changes:

  • Validate locales/*.json files are valid UTF-8 before base64-encoding and uploading.
  • Throw an AbortError that points to the offending locale file and explains how to fix it.
  • Add a unit test covering invalid UTF-8 byte sequences and include a patch changeset.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/app/src/cli/utilities/extensions/locales-configuration.ts Reads locale files as raw bytes, validates UTF-8, then base64-encodes; throws a helpful AbortError on invalid UTF-8.
packages/app/src/cli/utilities/extensions/locales-configuration.test.ts Adds coverage to ensure invalid UTF-8 locale files fail with a helpful error message that includes the filename.
.changeset/validate-locale-utf8.md Patch changeset describing the user-facing behavior change for early UTF-8 validation failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

Copy link
Copy Markdown
Contributor

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/error.d.ts
@@ -1,6 +1,6 @@
+import { AlertCustomSection } from './ui.js';
 import { OutputMessage } from './output.js';
 import { InlineToken, TokenItem } from '../../private/node/ui/components/TokenizedText.js';
-import type { AlertCustomSection } from './ui.js';
 export { ExtendableError } from 'ts-error';
 export declare enum FatalErrorType {
     Abort = 0,
packages/cli-kit/dist/public/node/is-global.d.ts
@@ -1,4 +1,4 @@
-import type { PackageManager } from './node-package-manager.js';
+import { PackageManager } from './node-package-manager.js';
 /**
  * Returns true if the current process is running in a global context.
  *
packages/cli-kit/dist/public/node/notifications-system.d.ts
@@ -24,7 +24,7 @@ declare const NotificationSchema: zod.ZodObject<{
     surface: zod.ZodOptional<zod.ZodString>;
 }, "strip", zod.ZodTypeAny, {
     id: string;
-    type: "info" | "error" | "warning";
+    type: "error" | "info" | "warning";
     message: string;
     frequency: "always" | "once" | "once_a_day" | "once_a_week";
     ownerChannel: string;
@@ -41,7 +41,7 @@ declare const NotificationSchema: zod.ZodObject<{
     surface?: string | undefined;
 }, {
     id: string;
-    type: "info" | "error" | "warning";
+    type: "error" | "info" | "warning";
     message: string;
     frequency: "always" | "once" | "once_a_day" | "once_a_week";
     ownerChannel: string;
@@ -84,7 +84,7 @@ declare const NotificationsSchema: zod.ZodObject<{
         surface: zod.ZodOptional<zod.ZodString>;
     }, "strip", zod.ZodTypeAny, {
         id: string;
-        type: "info" | "error" | "warning";
+        type: "error" | "info" | "warning";
         message: string;
         frequency: "always" | "once" | "once_a_day" | "once_a_week";
         ownerChannel: string;
@@ -101,7 +101,7 @@ declare const NotificationsSchema: zod.ZodObject<{
         surface?: string | undefined;
     }, {
         id: string;
-        type: "info" | "error" | "warning";
+        type: "error" | "info" | "warning";
         message: string;
         frequency: "always" | "once" | "once_a_day" | "once_a_week";
         ownerChannel: string;
@@ -120,7 +120,7 @@ declare const NotificationsSchema: zod.ZodObject<{
 }, "strip", zod.ZodTypeAny, {
     notifications: {
         id: string;
-        type: "info" | "error" | "warning";
+        type: "error" | "info" | "warning";
         message: string;
         frequency: "always" | "once" | "once_a_day" | "once_a_week";
         ownerChannel: string;
@@ -139,7 +139,7 @@ declare const NotificationsSchema: zod.ZodObject<{
 }, {
     notifications: {
         id: string;
-        type: "info" | "error" | "warning";
+        type: "error" | "info" | "warning";
         message: string;
         frequency: "always" | "once" | "once_a_day" | "once_a_week";
         ownerChannel: string;
packages/cli-kit/dist/public/node/hooks/postrun.d.ts
@@ -1,7 +1,3 @@
-/**
- * Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics)
- * at module evaluation time. These are only needed after the command has already finished.
- */
 import { Hook } from '@oclif/core';
 /**
  * Check if post run hook has completed.
packages/cli-kit/dist/public/node/hooks/prerun.d.ts
@@ -14,4 +14,4 @@ export declare function parseCommandContent(cmdInfo: {
  * Triggers a background check for a newer CLI version (non-blocking).
  * The result is cached and consumed by the postrun hook for auto-upgrade.
  */
-export declare function checkForNewVersionInBackground(): Promise<void>;
\ No newline at end of file
+export declare function checkForNewVersionInBackground(): void;
\ No newline at end of file

@amcaplan amcaplan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works as described. I would suggest making the changeset shorter.

I also suspect it would be easier for us users if we could link them to somewhere in the docs that explains the UTF-8 requirement. For now, I don't think such a link exists, which suggests we should have it in the docs somewhere if people are actually encountering this issue.

Comment thread .changeset/validate-locale-utf8.md Outdated
@github-actions github-actions Bot added the Area: @shopify/app @shopify/app package issues label Apr 28, 2026
@isaacroldan isaacroldan added this pull request to the merge queue Apr 29, 2026
Merged via the queue into main with commit abf8800 Apr 29, 2026
47 of 49 checks passed
@isaacroldan isaacroldan deleted the 04-27-fix_extension_locale_invalid_utf8_error branch April 29, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: @shopify/app @shopify/app package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants