[worker] Remove duplicated user-facing fields from build errors (#3469)

## Summary
- remove the duplicated `userFacingMessage` and `userFacingErrorCode` fields from the build error flow
- keep current runtime behavior and unknown-error semantics
- preserve the current worker protocol shape

Stanley: I don't want to have separate `userFacingMessage` and `message` / `userFacingErrorCode` and `errorCode`. It looks like the reason we had them separate was to log "UNKNOWN_ERROR" to the user if we encounter an internal issue like "NPM_CORRUPTED_PACKAGE". This pull request inverts the logic — we're going to have a `trackingCode` so we keep logging "NPM_CORRUPTED_PACKAGE" to Sentry and the default `errorCode` is the one we log to the user.

## Changes
- make `BuildError.format()` use canonical `message` and `errorCode`
- make `build-tools` store internal-only classification in `trackingCode` instead of parallel public fields
- make worker derive `internalErrorCode` from `trackingCode ?? errorCode`
- delete the obsolete `userFacing...` fields from `BuildError`

## Testing
- `packages/eas-build-job`: `npm exec -- corepack yarn test src/__tests__/errors.test.ts --runInBand`
- `packages/build-tools`: `npm exec -- corepack yarn jest-unit src/buildErrors/__tests__/detectError.test.ts --runInBand`
- `packages/worker`: `npm exec -- corepack yarn test:integration src/__integration__/stateSync.test.ts`

Author: sjchmiela

packages/build-tools/src/buildErrors/__tests__/detectError.test.ts

@@ -26,8 +26,11 @@ describe(resolveBuildPhaseErrorAsync, () => {
       },
       '/fake/path'
     );
-    expect(err.errorCode).toBe('NPM_CORRUPTED_PACKAGE');
-    expect(err.userFacingErrorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.errorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.message).toBe(
+      'Unknown error. See logs of the Install dependencies build phase for more information.'
+    );
+    expect(err.trackingCode).toBe('NPM_CORRUPTED_PACKAGE');
   });
 
   it('detects log for invalid bundler and reports it to user', async () => {
@@ -45,7 +48,10 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe('EAS_BUILD_UNSUPPORTED_BUNDLER_VERSION_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_UNSUPPORTED_BUNDLER_VERSION_ERROR');
+    expect(err.message).toBe(
+      'Your project requires a different version of the Ruby "bundler" program than the version installed in this EAS Build environment. You can specify which version of "bundler" to install by specifying the version under "build"→[buildProfileName]→"ios"→"bundler" in eas.json.'
+    );
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('does not detect errors if they show up in different build phase', async () => {
@@ -63,7 +69,10 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
-    expect(err.userFacingErrorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.message).toBe(
+      'Unknown error. See logs of the Install dependencies build phase for more information.'
+    );
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects npm cache error if cache is enabled', async () => {
@@ -82,8 +91,11 @@ describe(resolveBuildPhaseErrorAsync, () => {
       },
       '/fake/path'
     );
-    expect(err.errorCode).toBe('NPM_CACHE_ERROR');
-    expect(err.userFacingErrorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.errorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.message).toBe(
+      'Unknown error. See logs of the Install dependencies build phase for more information.'
+    );
+    expect(err.trackingCode).toBe('NPM_CACHE_ERROR');
   });
 
   it('does not detect npm cache error if cache is disabled', async () => {
@@ -103,7 +115,10 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
-    expect(err.userFacingErrorCode).toBe(errors.ErrorCode.UNKNOWN_ERROR);
+    expect(err.message).toBe(
+      'Unknown error. See logs of the Install dependencies build phase for more information.'
+    );
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects xcode line error', async () => {
@@ -126,7 +141,7 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/path/to/'
     );
     expect(err.errorCode).toBe('XCODE_RESOURCE_BUNDLE_CODE_SIGNING_ERROR');
-    expect(err.userFacingErrorCode).toBe('XCODE_RESOURCE_BUNDLE_CODE_SIGNING_ERROR');
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects minimum deployment target error correctly', async () => {
@@ -144,7 +159,7 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe('EAS_BUILD_HIGHER_MINIMUM_DEPLOYMENT_TARGET_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_HIGHER_MINIMUM_DEPLOYMENT_TARGET_ERROR');
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects provisioning profile mismatch error correctly', async () => {
@@ -162,7 +177,7 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe('EAS_BUILD_RESIGN_PROVISIONING_PROFILE_MISMATCH_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_RESIGN_PROVISIONING_PROFILE_MISMATCH_ERROR');
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects generic "Run Fastlane" error for resign correctly', async () => {
@@ -178,7 +193,7 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe('EAS_BUILD_UNKNOWN_FASTLANE_RESIGN_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_UNKNOWN_FASTLANE_RESIGN_ERROR');
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects build error in "Run Fastlane" phase correctly', async () => {
@@ -194,7 +209,7 @@ describe(resolveBuildPhaseErrorAsync, () => {
       '/fake/path'
     );
     expect(err.errorCode).toBe('EAS_BUILD_UNKNOWN_FASTLANE_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_UNKNOWN_FASTLANE_ERROR');
+    expect(err.trackingCode).toBeUndefined();
   });
 
   it('detects provisioning profile mismatch error correctly', async () => {
@@ -240,8 +255,8 @@ note`;
       '/path/to'
     );
     expect(err.errorCode).toBe('XCODE_BUILD_ERROR');
-    expect(err.userFacingErrorCode).toBe('XCODE_BUILD_ERROR');
-    expect(err.userFacingMessage)
+    expect(err.trackingCode).toBeUndefined();
+    expect(err.message)
       .toBe(`The "Run fastlane" step failed because of an error in the Xcode build process. We automatically detected following errors in your Xcode build logs:
 - The signature of “ProgrammaticAccessLibrary.xcframework” cannot be verified.
 - Some other error
@@ -263,9 +278,9 @@ Refer to "Xcode Logs" below for additional, more detailed logs.`);
       '/fake/path'
     );
 
-    expect(err.errorCode).toBe('MAVEN_CACHE_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_UNKNOWN_GRADLE_ERROR');
-    expect(err.userFacingMessage).toBe(
+    expect(err.errorCode).toBe('EAS_BUILD_UNKNOWN_GRADLE_ERROR');
+    expect(err.trackingCode).toBe('MAVEN_CACHE_ERROR');
+    expect(err.message).toBe(
       `Gradle build failed with unknown error. See logs for the "Run gradlew" phase for more information.`
     );
   });
@@ -285,8 +300,8 @@ Refer to "Xcode Logs" below for additional, more detailed logs.`);
     );
 
     expect(err.errorCode).toBe('EAS_BUILD_UNKNOWN_GRADLE_ERROR');
-    expect(err.userFacingErrorCode).toBe('EAS_BUILD_UNKNOWN_GRADLE_ERROR');
-    expect(err.userFacingMessage).toBe(
+    expect(err.trackingCode).toBeUndefined();
+    expect(err.message).toBe(
       `Gradle build failed with unknown error. See logs for the "Run gradlew" phase for more information.`
     );
   });

packages/build-tools/src/buildErrors/detectError.ts

@@ -80,25 +80,14 @@ export async function resolveBuildPhaseErrorAsync(
         new errors.UnknownError(errorContext.phase));
   const buildError = resolveError(buildErrorHandlers, logLines, errorContext, xcodeBuildLogs);
 
-  const isUnknownUserError =
-    !userFacingError ||
-    (
-      [
-        errors.ErrorCode.UNKNOWN_ERROR,
-        errors.ErrorCode.UNKNOWN_GRADLE_ERROR,
-        errors.ErrorCode.UNKNOWN_FASTLANE_ERROR,
-      ] as string[]
-    ).includes(userFacingError.errorCode);
-  const message =
-    (isUnknownUserError ? buildError?.message : userFacingError.message) ?? userFacingError.message;
-  const errorCode =
-    (isUnknownUserError ? buildError?.errorCode : userFacingError.errorCode) ??
-    userFacingError.errorCode;
+  const trackingCode =
+    buildError && buildError.errorCode !== userFacingError.errorCode
+      ? buildError.errorCode
+      : undefined;
 
-  return new errors.BuildError(message, {
-    errorCode,
-    userFacingErrorCode: userFacingError.errorCode,
-    userFacingMessage: userFacingError.message,
+  return new errors.BuildError(userFacingError.message, {
+    errorCode: userFacingError.errorCode,
+    trackingCode,
     docsUrl: userFacingError.docsUrl,
     innerError: error,
     buildPhase: phase,

packages/build-tools/src/context.ts

@@ -299,7 +299,7 @@ export class BuildContext<TJob extends Job = Job> {
       // leaving message empty, website will display err.stack which already includes err.message
       this.logger.error({ err }, '');
     } else {
-      this.logger.error(`Error: ${buildError.userFacingMessage}`);
+      this.logger.error(`Error: ${buildError.message}`);
     }
     return buildError;
   }

packages/eas-build-job/src/__tests__/errors.test.ts

@@ -1,4 +1,22 @@
-import { UserFacingError } from '../errors';
+import { BuildError, UserFacingError } from '../errors';
+import { BuildPhase } from '../logs';
+
+describe(BuildError, () => {
+  it('formats using canonical message and errorCode', () => {
+    const error = new BuildError('canonical message', {
+      errorCode: 'ERR_CODE',
+      docsUrl: 'https://docs.example.dev',
+      buildPhase: BuildPhase.PREBUILD,
+    });
+
+    expect(error.format()).toEqual({
+      errorCode: 'ERR_CODE',
+      message: 'canonical message',
+      docsUrl: 'https://docs.example.dev',
+      buildPhase: BuildPhase.PREBUILD,
+    });
+  });
+});
 
 describe(UserFacingError, () => {
   it('supports docsUrl in options', () => {

packages/eas-build-job/src/errors.ts

@@ -19,34 +19,33 @@ export interface ExternalBuildError {
 
 interface BuildErrorDetails {
   errorCode: string;
-  userFacingMessage: string;
-  userFacingErrorCode: string;
+  trackingCode?: string;
   docsUrl?: string;
   innerError?: Error;
   buildPhase?: BuildPhase;
 }
 
 export class BuildError extends Error {
   public errorCode: string;
-  public userFacingMessage: string;
-  public userFacingErrorCode: string;
+  // Internal-only classification used for Sentry, analytics, and worker internalErrorCode.
+  // The public error saved on builds and job runs is always `errorCode`.
+  public trackingCode?: string;
   public docsUrl?: string;
   public innerError?: Error;
   public buildPhase?: BuildPhase;
 
   constructor(message: string, details: BuildErrorDetails) {
     super(message);
     this.errorCode = details.errorCode;
-    this.userFacingErrorCode = details.userFacingErrorCode;
-    this.userFacingMessage = details.userFacingMessage;
+    this.trackingCode = details.trackingCode;
     this.docsUrl = details.docsUrl;
     this.innerError = details.innerError;
     this.buildPhase = details.buildPhase;
   }
   public format(): ExternalBuildError {
     return {
-      errorCode: this.userFacingErrorCode,
-      message: this.userFacingMessage,
+      errorCode: this.errorCode,
+      message: this.message,
       docsUrl: this.docsUrl,
       buildPhase: this.buildPhase,
     };
@@ -83,8 +82,6 @@ export class UnknownBuildError extends BuildError {
     const message = 'Unknown error. See logs for more information.';
     super(message, {
       errorCode,
-      userFacingMessage: message,
-      userFacingErrorCode: errorCode,
     });
   }
 }
@@ -95,8 +92,6 @@ export class UnknownCustomBuildError extends BuildError {
     const message = 'Unknown custom build error. See logs for more information.';
     super(message, {
       errorCode,
-      userFacingMessage: message,
-      userFacingErrorCode: errorCode,
     });
   }
 }

packages/worker/src/__integration__/stateSync.test.ts

@@ -417,5 +417,76 @@ describe('State sync mechanism', () => {
       ws.close();
       clearTimeout(messageTimeout);
     });
+
+    it('should expose tracking code as internal error code from generic job', async () => {
+      jest.mocked(runGenericJobAsync).mockResolvedValueOnce({
+        runResult: result(
+          new errors.BuildError('ASC app was not found for this account.', {
+            errorCode: genericJobRunErrorCode,
+            trackingCode: 'GENERIC_TRACKING_ERROR',
+          })
+        ),
+        buildWorkflow: {} as any,
+      });
+      const dispatchWS = new WebSocket(`ws://localhost:${port}?expo_vm_name=${hostname()}`);
+      const dispatchHelper = new WsHelper(dispatchWS);
+      await dispatchHelper.onOpen();
+      dispatchWS.send(
+        JSON.stringify({
+          type: 'dispatch',
+          buildId,
+          job: createTestGenericJob(),
+          initiatingUserId: '14367e1b-26fc-4c00-aedb-0629d78f8286',
+          metadata: {
+            trackingContext: {},
+          },
+        })
+      );
+      dispatchWS.close();
+      await dispatchHelper.onClose();
+
+      const ws = new WebSocket(`ws://localhost:${port}?expo_vm_name=${hostname()}`);
+      const helper = new WsHelper(ws);
+
+      let stateResponsePromiseResolve: () => void;
+      const stateResponsePromise = new Promise<void>(res => {
+        stateResponsePromiseResolve = res;
+      });
+      const onMessage = jest.fn((message: any) => {
+        clearTimeout(messageTimeout);
+        try {
+          expect(message).toBeTruthy();
+          expect(message.type).toBe('state-response');
+          expect(message.status).toBe('error');
+          expect(message.externalBuildError?.errorCode).toBe(genericJobRunErrorCode);
+          expect(message.externalBuildError?.message).toContain(
+            'ASC app was not found for this account.'
+          );
+          expect(message.internalErrorCode).toBe('GENERIC_TRACKING_ERROR');
+        } catch (err) {
+          throw err;
+        } finally {
+          stateResponsePromiseResolve();
+        }
+      });
+      const openPromise = helper.onOpen();
+      helper.onMessage(onMessage);
+
+      await openPromise;
+      messageTimeout = setTimeout(() => {
+        unreachableCode('state-response timeout');
+      }, 3000);
+
+      ws.send(JSON.stringify({ type: 'state-query', buildId }));
+
+      await stateResponsePromise;
+
+      expect(helper.onErrorCb).not.toHaveBeenCalled();
+      expect(helper.onOpenCb).toHaveBeenCalled();
+      expect(helper.onMessageCb).toHaveBeenCalled();
+      expect(helper.onCloseCb).not.toHaveBeenCalled();
+      ws.close();
+      clearTimeout(messageTimeout);
+    });
   });
 });

packages/worker/src/build.ts

@@ -104,15 +104,16 @@ function logBuildError(logger: bunyan, analytics: Analytics, err: Error): void {
   l.info({ marker: LogMarker.START_PHASE }, `Start phase: ${BuildPhase.FAIL_BUILD}`);
 
   if (err instanceof errors.BuildError) {
+    const internalErrorCode = err.trackingCode ?? err.errorCode;
     analytics.logEvent(Event.WORKER_BUILD_FAIL, {
       reason: err?.message,
-      error_code: err.errorCode,
+      error_code: internalErrorCode,
       build_phase: err.buildPhase,
     });
     if (err.errorCode !== errors.ErrorCode.UNKNOWN_ERROR) {
-      l.error(`Build failed: ${err.userFacingMessage}`);
+      l.error(`Build failed: ${err.message}`);
     } else {
-      l.error({ err: err.innerError ?? err.userFacingMessage }, `Build failed\n`);
+      l.error({ err: err.innerError ?? err }, `Build failed\n`);
     }
   } else {
     // This can only happen when error is thrown outside of a build phase.