fix: Brittle response parsing from notarytool by connorgmeehan · Pull Request #191 · electron/notarize

connorgmeehan · 2024-04-30T03:07:26Z

Description

Electron builder is not handling responses from notarytool on my device (MacOS Sonoma 14.4.1, XCode 15.2). The issue is it's trying to JSON.parse a string with the value:

Error: HTTP status code: 401. Invalid credentials. Username or password is incorrect. Use the app-specific password generated at appleid.apple.com. Ensure that all authentication arguments are correct.

Solution

Fix the ordering of the status code check for the result of spawn
For extra safety, catch errors when JSON.parse fails.

Compared log when errors

Previous logs

• electron-builder  version=24.13.3 os=23.4.0
  • loaded configuration  file=/Users/connormeehan/projects/eyejack/eyejack-creator/electron-builder.config.js
  • description is missed in the package.json  appPackageFile=/Users/connormeehan/projects/eyejack/eyejack-creator/dist_electron/bundled/package.json
  • author is missed in the package.json  appPackageFile=/Users/connormeehan/projects/eyejack/eyejack-creator/dist_electron/bundled/package.json
  • writing effective config  file=dist_electron/bundled/dist/builder-effective-config.yaml
  • rebuilding native dependencies  dependencies=sharp@0.32.6 platform=darwin arch=x64
  • install prebuilt binary  name=sharp version=0.32.6 platform=darwin arch=x64 napi=
                                                                                       • packaging       platform=darwin arch=x64 electron=14.2.9 appOutDir=dist_electron/bundled/dist/mac
  • signing         file=dist_electron/bundled/dist/mac/EyeJack Creator 2024.app platform=darwin type=distribution identity=34E41E43ADF5846B288472E3BA659E71DC2369DB provisioningProfile=none
  ⨯ Unexpected token 'E', "Error: HTT"... is not valid JSON  failedTask=build stackTrace=SyntaxError: Unexpected token 'E', "Error: HTT"... is not valid JSON
SyntaxError: Unexpected token 'E', "Error: HTT"... is not valid JSON
    at JSON.parse (<anonymous>)
    at /Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/node_modules/@electron/notarize/src/notarytool.ts:79:25
    at Generator.next (<anonymous>)
    at fulfilled (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/node_modules/@electron/notarize/lib/notarytool.js:28:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
From previous event:
    at processImmediate (node:internal/timers:478:21)
From previous event:
    at readDirectoryAndSign (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/macPackager.ts:478:29)
    at MacPackager.signApp (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/macPackager.ts:488:11)
    at MacPackager.doSignAfterPack (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/platformPackager.ts:336:21)
    at MacPackager.doPack (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/platformPackager.ts:321:7)
    at MacPackager.pack (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/macPackager.ts:198:9)
    at Packager.doBuild (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/packager.ts:445:9)
    at executeFinally (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/builder-util/src/promise.ts:12:14)
    at Packager._build (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/packager.ts:379:31)
    at Packager.build (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/app-builder-lib/src/packager.ts:340:12)
    at executeFinally (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/builder-util/src/promise.ts:12:14)

New logs

• electron-builder  version=24.13.3 os=23.4.0
  • loaded configuration  file=/Users/connormeehan/projects/eyejack/eyejack-creator/electron-builder.config.js
  • description is missed in the package.json  appPackageFile=/Users/connormeehan/projects/eyejack/eyejack-creator/dist_electron/bundled/package.json
  • author is missed in the package.json  appPackageFile=/Users/connormeehan/projects/eyejack/eyejack-creator/dist_electron/bundled/package.json
  • writing effective config  file=dist_electron/bundled/dist/builder-effective-config.yaml
  • rebuilding native dependencies  dependencies=sharp@0.32.6 platform=darwin arch=x64
  • install prebuilt binary  name=sharp version=0.32.6 platform=darwin arch=x64 napi=
                                                                                       • packaging       platform=darwin arch=x64 electron=14.2.9 appOutDir=dist_electron/bundled/dist/mac
  • signing         file=dist_electron/bundled/dist/mac/EyeJack Creator 2024.app platform=darwin type=distribution identity=34E41E43ADF5846B288472E3BA659E71DC2369DB provisioningProfile=none
  • skipped macOS notarization  reason=`notarize` options were set explicitly `false`
  ⨯ Failed to notarize via notarytool

Error: HTTP status code: 401. Invalid credentials. Username or password is incorrect. Use the app-specific password generated at appleid.apple.com. Ensure that all authentication arguments are correct.
  failedTask=build stackTrace=Error: Failed to notarize via notarytool

                     Error: HTTP status code: 401. Invalid credentials. Username or password is incorrect. Use the app-specific password generated at appleid.apple.com. Ensure that all authentication arguments are correct.

                         at /Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/@electron/notarize/src/notarytool.ts:87:13
    at Generator.next (<anonymous>)
    at fulfilled (/Users/connormeehan/projects/eyejack/eyejack-creator/node_modules/@electron/notarize/lib/notarytool.js:28:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

rotu · 2024-05-07T16:20:38Z

Thanks for taking this on! It's a huge improvement by not truncating or swallowing the non-JSON text.

rotu · 2024-05-08T17:14:50Z

I'm not sure what the return code actually tells us. Does a return code of 0 mean notarization succeeded? Does it mean that the request was submitted correctly (and may have failed)?

Here are the four cases to consider:

exit code non-zero, non-JSON output (the problem case): Failure. Report text output as-is.
exit code non-zero, JSON output: (I originally thought this wouldn't happen. Now I think it could)
exit code zero, non-JSON output: I still think this is impossible.
exit code zero, JSON output. I think the output of notarytool log may contain non-error warnings so it should be printed regardless.

connorgmeehan · 2024-05-08T23:21:37Z

From a little testing here's what I found

Happy path: Uploads and notarizes successfully. Returns JSON with exit-code 0
ERR: Uploads but invalid. Return JSON with exit-code !== 0.
ERR: Wrong arguments to notarytool (not uploaded). Returns text with exit-code !== 0 (The issue this PR addresses)

❯ xcrun notarytool submit ./EyeJack\ Creator\ 2024-3.4.2.dmg  ....AUTH_ARGS... -f json --wait
{"status":"Accepted","id":"fa15ba57-3b66-4934-b7d1-937996f94f05","message":"Processing complete"}

❯ echo $?
0

❯ xcrun notarytool submit ./EyeJack\ Creator\ 2024-3.4.2-mac.zip ...AUTH_ARGS...  -f json --wait --force
{"id":"beb99749-fd68-4d25-ba38-1a6a343ae96e","status":"Invalid","message":"Processing complete"}

❯ echo $?
130

Does a return code of 0 mean notarization succeeded?

Yep, seems to be

rotu

Looks awesome to me :-)

…n#195) Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>

QuentinMacheda · 2024-05-14T16:18:24Z

I hope this fix will be released soon, I'm stuck not knowing the cause of the error I get...

rotu · 2024-05-14T16:50:16Z

@jonluca could you get this out the door or get another active maintainer's eyes on it please?

…otarize into connorgmeehan-fix/print-notarize-error

continuous-auth · 2024-05-14T18:59:50Z

🎉 This PR is included in version 2.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

fix: Brittle response parsing from notarytool

853570d

connorgmeehan requested a review from a team as a code owner April 30, 2024 03:07

fix: Fixes result.code check blocking notarytool log diagnostics

a7698ef

connorgmeehan marked this pull request as draft April 30, 2024 03:31

connorgmeehan marked this pull request as ready for review May 6, 2024 00:32

cpvalente mentioned this pull request May 7, 2024

Unexpected token E in JSON at position 0 #186

Closed

rotu reviewed May 7, 2024

View reviewed changes

Comment thread src/notarytool.ts Outdated

fix: Concat notarytool error with notarytool log result

514cb3d

connorgmeehan requested a review from rotu May 8, 2024 08:22

clean: Cleanup unused import

77eda12

rotu reviewed May 8, 2024

View reviewed changes

Comment thread src/notarytool.ts Outdated

rotu reviewed May 8, 2024

View reviewed changes

Comment thread src/notarytool.ts Outdated

refact: Simplified notary error handling logic

195de70

connorgmeehan force-pushed the fix/print-notarize-error branch from 5dc95c6 to 195de70 Compare May 8, 2024 23:29

connorgmeehan requested a review from rotu May 8, 2024 23:30

rotu reviewed May 9, 2024

View reviewed changes

chore: bump electronjs/node in .circleci/config.yml to 2.2.2 (electro…

ba33a3d

…n#195) Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>

erickzhao assigned MarshallOfSound May 14, 2024

MarshallOfSound added 2 commits May 14, 2024 11:46

Merge branch 'fix/print-notarize-error' of github.com:connorgmeehan/n…

fe55db4

…otarize into connorgmeehan-fix/print-notarize-error

fix: ensure status is Accepted

7b49573

MarshallOfSound removed their assignment May 14, 2024

MarshallOfSound approved these changes May 14, 2024

View reviewed changes

VerteDinde approved these changes May 14, 2024

View reviewed changes

erickzhao approved these changes May 14, 2024

View reviewed changes

MarshallOfSound merged commit 8440fa1 into electron:main May 14, 2024

continuous-auth Bot added the released label May 14, 2024

rotu mentioned this pull request May 15, 2024

Plain text error output from xcrun is not handled #177

Closed

kochie mentioned this pull request Jun 23, 2024

fix(deps): Update notarize tool electron-userland/electron-builder#8271

Merged

This was referenced Sep 8, 2024

[Snyk] Upgrade: , , minimatch, tar, , hosted-git-info, resedit Playgirlkaybraz11/electron-builder#9

Open

[Snyk] Upgrade: , , Playgirlkaybraz11/electron-builder#11

Open

Navigation Menu

fix: Brittle response parsing from notarytool - #191

fix: Brittle response parsing from notarytool#191
MarshallOfSound merged 8 commits into
electron:mainfrom
connorgmeehan:fix/print-notarize-error

connorgmeehan commented Apr 30, 2024 •

edited

Loading

Uh oh!

rotu commented May 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rotu commented May 8, 2024

Uh oh!

connorgmeehan commented May 8, 2024 •

edited

Loading

Uh oh!

rotu left a comment

Uh oh!

QuentinMacheda commented May 14, 2024

Uh oh!

rotu commented May 14, 2024

Uh oh!

continuous-auth Bot commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

connorgmeehan commented Apr 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Compared log when errors

Previous logs

New logs

Uh oh!

rotu commented May 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rotu commented May 8, 2024

Uh oh!

connorgmeehan commented May 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rotu left a comment

Choose a reason for hiding this comment

Uh oh!

QuentinMacheda commented May 14, 2024

Uh oh!

rotu commented May 14, 2024

Uh oh!

continuous-auth Bot commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

connorgmeehan commented Apr 30, 2024 •

edited

Loading

connorgmeehan commented May 8, 2024 •

edited

Loading