[fastlane-core] recommends to retry uploading when AltoolTransporterExecutor crashes #21536
Conversation
TheMetalCode
changed the title
Co-authored-by: Aaron Brager <getaaron@gmail.com>
…lCode/fastlane into jjh/ensure-pty-returns-exit-code
|
Isn't the error that spawn returns a Looking at fastlane/fastlane_core/lib/fastlane_core/fastlane_pty.rb Lines 22 to 55 in 34a1635 The last We could instead make the Pty classes more resilient and return |
|
I've made #21618 to handle the cases of a program crashing. In that case These extra lines shouldn't be necessary. |
@lacostej To be clear: I was never confident that this code ever solved the issue with Today I have tried using #21618 instead of this code and it seems to work fine. The only hesitation being that since moving up to Xcode 15.0.1 I'm no longer seeing my pilot actions fail, so I cannot say for sure that your code handles the failure any better than mine. 🤷♂️ I'm a big fan of the added testing in #21618 so for that reason alone I'm totally fine if this PR closes and yours gets merged instead. |
I think we should merge both PRs
Once #21618 is merged, I also think you can remove the catch for |
| @@ -215,6 +215,8 @@ class AltoolTransporterExecutor < TransporterExecutor | |||
|
|
|||
| private_constant :ERROR_REGEX | |||
|
|
|||
| attr_reader :errors | |||
There was a problem hiding this comment.
This seems safe, it makes testing the specific error line we're adding for the -1 return more straightforward
|
Saw that #21618 merged. In response, I've removed the nil catch in |
lacostej
changed the title
…xecutor crashes (fastlane#21536) * Force FastlanePty.spawn to return exit code if it ends up being nil for whatever reason * Actually, let's implement the fix right where the problem occurs, and add a test * Update fastlane_core/lib/fastlane_core/itunes_transporter.rb Co-authored-by: Aaron Brager <getaaron@gmail.com> * Remove unneeded nil catch given fastlane#21618 * Write more accurate test --------- Co-authored-by: Aaron Brager <getaaron@gmail.com>
Checklist
bundle exec rspecfrom the root directory to see all new and existing tests passbundle exec rubocop -ato ensure the code style is validci/circlecibuilds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)Motivation and Context
Resolves #21535
After recently noticing some of my
pilotruns fail due to no exit code being returned when runningaltoolviaFastlanePty.spawn, I investigated and eventually realized the error handling inAltoolTransporterExecutor#executewill work as designed if we simply force the return of some kind of exit code when this mysterious issue withaltooloccurs.I do not know why
altoolis failing in this manner and hence this change isn't exactly a solution. What it does do though is provide a better error output when this condition does occur, such that it's much easier for a fastlane user to detect this precise failure and automatically retry the upload (which in my experience over the last few days always works successfully). With this change, paired with settingDELIVER_ALTOOL_ADDITIONAL_UPLOAD_PARAMETERS="--verbose"in the environment soaltoolis guaranteed to have some kind of stdout, the rather cryptic error output in #21535 turns into something much more informative:Description
Before,
AltoolTransporterExecutor#executesimply attempted to assertexit_code.zero?on the assumption that it was being given an integer exit code from the command when executed viaFastlanePty.spawn. Now, it forces a-1if it receives anilfromFastlanePty.spawn.Testing Steps