Skip to content

Rerun Publish Steps after Network Error#12413

Merged
jikim-msft merged 24 commits into
mainfrom
test/rerun-steps-network-error
Oct 16, 2022
Merged

Rerun Publish Steps after Network Error#12413
jikim-msft merged 24 commits into
mainfrom
test/rerun-steps-network-error

Conversation

@jikim-msft
Copy link
Copy Markdown
Contributor

@jikim-msft jikim-msft commented Oct 11, 2022

Description

665: Ability to Rerun Publish Step If There Are Any Network Error https://dev.azure.com/fluidframework/internal/_workitems/edit/665/

Cases

  1. If Publish Package does not encounter an error continue with the remaining packages

  2. If Publish Package encounters an error

  • Network Error: retry for $maximumRetryIfNetworkError-amount of time. If network error is persisted during the entire loop, exit the task in the pipeline
  • Non-Network Error: Such as the current package is already published in the registry, fail the pipeline and not continue

@jikim-msft jikim-msft requested a review from a team as a code owner October 11, 2022 23:55
@github-actions github-actions Bot added area: build Build related issues base: main PRs targeted against main branch labels Oct 11, 2022
let t1+=1
if ! grep -q "You cannot publish over the previously published versions" "publish_log"
then
let t1+=1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should fail and not continue in this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed it by moving code below line 140 (any error case besides the network error)

  fi
  rm "$f"
  mv "$local_f" "$f"
  exit $t1
fi

fi
rm "$f"
mv "$local_f" "$f"
exit $t1
Copy link
Copy Markdown
Member

@curtisman curtisman Oct 12, 2022

Choose a reason for hiding this comment

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

This seems to exit even if the content of the package is the same?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an existing behavior prior to working on the codebase. I have switched the non-network error case to the else case of the if grep -q "code ENOTFOUND" "publish_log"

Comment thread tools/pipelines/templates/include-publish-npm-package-steps.yml
else
echo ERROR: Published package does not match the current one attempting to be released for the same version.
let t1+=1
if ! grep -q "You cannot publish over the previously published versions" "publish_log"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How do you get this error message. Did you try it in npmjs.org?

ADO feed might have a different error message. See example

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This error message existed before I started working on the codebase. The npm error message that I added is grep -q "code ENOTFOUND" which appears when there is a timeout during publishing the package to the npm registry.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please test your change to make sure it works.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NPM_Error

When disconnecting network in the middle of npm publish, npm ERR! code ENOTFOUND error message is returned.

Created a dummy package npm-return-error-msg which invokes the identical error message and checked that the process repeats for x-amount of times.

@jikim-msft jikim-msft requested a review from curtisman October 12, 2022 00:49
npm pack $package_name
if cmp -s "$f" "$local_f"
then
echo Continuing as published package matches the current one that was attempting to be released.
Copy link
Copy Markdown
Member

@curtisman curtisman Oct 12, 2022

Choose a reason for hiding this comment

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

We are still exiting even if the packages are the same?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added continue in line 131 to continue the loop if the packages are the same.

echo Continuing as published package matches the current one that was attempting to be released.
else
echo ERROR: Published package does not match the current one attempting to be released for the same version.
let t1+=1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

t1 used be used for tracking how many errors we encountered, but since we are going to exist on error now, we probably don't need it anymore.

Copy link
Copy Markdown
Contributor Author

@jikim-msft jikim-msft Oct 12, 2022

Choose a reason for hiding this comment

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

Removed t1 from the codebase and made exit status to exit 1

As a result, made changes to the existing if-statement

Before

else
    if ! grep -q "You cannot publish over the previously published versions" "publish_log"
    then
        let t1+=1
     else
         echo Package has already been published.
          local_f="${f}_local"
          mv "$f" "$local_f"
          package_name=$(grep -oP 'npm notice package: \K.*' "publish_log")
          npm pack $package_name
          if cmp -s "$f" "$local_f"
          then
                 echo Continuing as published package matches the current one that was attempting to be released.
          else
                echo ERROR: Published package does not match the current one attempting to be released for the same version.
                 let t1+=1
         fi
      fi
    rm "$f"
    mv "$local_f" "$f"
    exit $t1
  fi

After

else
    if grep -q "You cannot publish over the previously published versions" "publish_log"
    then
         echo Package has already been published.
          local_f="${f}_local"
          mv "$f" "$local_f"
          package_name=$(grep -oP 'npm notice package: \K.*' "publish_log")
          npm pack $package_name
          if cmp -s "$f" "$local_f"
          then
                 echo Continuing as published package matches the current one that was attempting to be released.
                 continue
          else
                echo ERROR: Published package does not match the current one attempting to be released for the same version.
           fi
      fi
    rm "$f"
    mv "$local_f" "$f"
    exit $t1
  fi

@jikim-msft jikim-msft merged commit 7e25b3e into main Oct 16, 2022
@jikim-msft jikim-msft deleted the test/rerun-steps-network-error branch October 16, 2022 03:17
sharptrip pushed a commit to sharptrip/FluidFramework that referenced this pull request Oct 17, 2022
### Description
665: Ability to Rerun Publish Step If There Are Any Network Error
https://dev.azure.com/fluidframework/internal/_workitems/edit/665/

### Cases
1. If Publish Package does not encounter an error continue with the
remaining packages

2. If Publish Package encounters an error
- Network Error: retry for `$maximumRetryIfNetworkError`-amount of time.
If network error is persisted during the entire loop, exit the task in
the pipeline
- Non-Network Error: Such as the current package is already published in
the registry, fail the pipeline and not continue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: build Build related issues base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants