Improve reporting of errors & efficiency of pr-size-labeler (#7288)

* Surface curl errors

Errors in the invocation of `curl` were not being reported. Added `-f`
flag to `curl`, return status checking, and printing the response body
in case of errors.

* Use echo, not return

* Adjust style of variable references to match existing code

* Fix a couple of mistakes and improve DRYness

- `cat` should have been `echo`
- remove debugging setting
- refactor a few lines in api_call()

* Get rid of token parameter for workflow_dispatch

I never seem to need it.

* Add option to set Bash tracing

This approach is a simple way to debug all shell scripts and commands.

* Speed up git checkouts by making them sparse

* Clean up temporary files created in jq_stdin

And make sure it still returns the jq status code.

* Rewrite api_call to output response from the server when call fails

Write error response to stderr as stdout may be captured by the caller.
Use `cat` for outputting response as `echo` may translate escape sequences.
Make sure api_call returns the curl status code.

* Terminate early if api_call returns error status

Avoid using `cmd <<<"$(api_call args)"` expressions which ignore
`api_call` failure.

* Do not discard stderr from api_call

This is where it sends diagnostics if any.

* Switch to a PAT instead of GITHUB_TOKEN

Once again, got bitten by the fact that adding labels to an issue
doesn't work with `GITHUB_TOKEN`

C.f. https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#add-labels-to-an-issue

* Add comments to explain some non-obvious parts

This is for future readers.

* Take out pipe

Per [review comments by
@Pavol](#7288 (comment)),
it is not doing what I thought it was doing. Plus, we don't really need the
final output step.

* Go back to using pull_request_target & GITHUB_TOKEN

---------

Co-authored-by: Pavol Juhas <juhas@google.com>

Author: mhucka

.github/workflows/pr-labeler.yaml

@@ -25,7 +25,10 @@ run-name: >-
   Label pull request ${{github.event.pull_request.number}} by ${{github.actor}}
 
 on:
-  pull_request:
+  # Note: do not copy-paste this workflow with `pull_request_target` left as-is.
+  # Its use here is a special case where security implications are understood.
+  # Workflows should normally use `pull_request` instead.
+  pull_request_target:
     types:
       - opened
       - synchronize
@@ -37,9 +40,9 @@ on:
         description: 'The PR number of the PR to label:'
         type: string
         required: true
-      token:
-        description: 'Token to use for authentication:'
-        type: string
+      trace:
+        description: 'Turn on shell script debugging'
+        type: boolean
 
 # Declare default permissions as read only.
 permissions: read-all
@@ -55,23 +58,19 @@ jobs:
       pull-requests: write
     env:
       PR_NUMBER: ${{inputs.pr-number || github.event.pull_request.number}}
+      # The next var is used by Bash. We add 'xtrace' to the options if this run
+      # is a workflow_dispatch invocation and the user set the 'trace' option.
+      SHELLOPTS: ${{inputs.trace && 'xtrace' || '' }}
     steps:
       - name: Check out a copy of the git repository
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          sparse-checkout: |
+            ./dev_tools/ci/size-labeler.sh
 
       - name: Label the PR with a size label
         id: label
         continue-on-error: true
         env:
-          GITHUB_TOKEN: ${{inputs.token || github.token}}
-        run: ./dev_tools/ci/size-labeler.sh |& tee action.out
-
-      - name: Detect and report errors in labeler action
-        if: steps.label.outcome == 'failure' || steps.label.outcome == 'error'
-        run: |
-          {
-          echo "<h4>‼️ Failed to label PR ${PR_NUMBER}</h4>"
-          echo "<pre>"
-          cat action.out
-          echo "</pre>"
-          } >> "$GITHUB_STEP_SUMMARY"
+          GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
+        run: ./dev_tools/ci/size-labeler.sh

dev_tools/ci/size-labeler.sh

@@ -59,9 +59,12 @@ function jq_stdin() {
     local infile
     infile="$(mktemp)"
     readonly infile
+    local jq_status=0
 
     cat >"${infile}"
-    jq_file "$@" "${infile}"
+    jq_file "$@" "${infile}" || jq_status="${?}"
+    rm "${infile}"
+    return "${jq_status}"
 }
 
 function jq_file() {
@@ -81,22 +84,38 @@ function jq_file() {
 function api_call() {
     local -r endpoint="${1// /%20}" # love that our label names have spaces...
     local -r uri="https://api.github.com/repos/${GITHUB_REPOSITORY}"
+    local response
+    local curl_status=0
     info "Calling: ${uri}/${endpoint}"
-    curl -sSL \
+    response="$(curl -sSL \
+        --fail-with-body \
+        --connect-timeout 10 --max-time 20 \
         -H "Authorization: token ${GITHUB_TOKEN}" \
         -H "Accept: application/vnd.github.v3.json" \
         -H "X-GitHub-Api-Version:2022-11-28" \
         -H "Content-Type: application/json" \
         "${@:2}" \
         "${uri}/${endpoint}"
+    )" || curl_status="${?}"
+    if [[ -n "${response}" ]]; then
+        cat <<<"${response}"
+    fi
+    if (( curl_status )); then
+        error "GitHub API call failed (curl exit $curl_status) for ${uri}/${endpoint}"
+        error "Response body:"
+        cat >&2 <<<"${response}"
+    fi
+    return "${curl_status}"
 }
 
 function compute_changes() {
     local -r pr="$1"
 
+    local response
     local change_info
     local -r keys_filter='with_entries(select([.key] | inside(["changes", "filename"])))'
-    change_info="$(jq_stdin "map(${keys_filter})" <<<"$(api_call "pulls/${pr}/files")")"
+    response="$(api_call "pulls/${pr}/files")"
+    change_info="$(jq_stdin "map(${keys_filter})" <<<"${response}")"
 
     local files total_changes
     readarray -t files < <(jq_stdin -c '.[]' <<<"${change_info}")
@@ -131,8 +150,10 @@ function get_size_label() {
 function prune_stale_labels() {
     local -r pr="$1"
     local -r size_label="$2"
+    local response
     local existing_labels
-    existing_labels="$(jq_stdin -r '.labels[] | .name' <<<"$(api_call "pulls/${pr}")")"
+    response="$(api_call "pulls/${pr}")"
+    existing_labels="$(jq_stdin -r '.labels[] | .name' <<<"${response}")"
     readarray -t existing_labels <<<"${existing_labels}"
 
     local correctly_labeled=false
@@ -147,7 +168,7 @@ function prune_stale_labels() {
         # If there is another size label, we need to remove it
         if [[ -v "LIMITS[${label}]" ]]; then
             info "Label '${label}' is stale, removing it."
-            api_call "issues/${pr}/labels/${label}" -X DELETE &>/dev/null
+            api_call "issues/${pr}/labels/${label}" -X DELETE >/dev/null
             continue
         fi
         info "Label '${label}' is unknown, leaving it."
@@ -194,7 +215,7 @@ function main() {
     correctly_labeled="$(prune_stale_labels "$PR_NUMBER" "${size_label}")"
 
     if [[ "${correctly_labeled}" != true ]]; then
-        api_call "issues/$PR_NUMBER/labels" -X POST -d "{\"labels\":[\"${size_label}\"]}" &>/dev/null
+        api_call "issues/$PR_NUMBER/labels" -X POST -d "{\"labels\":[\"${size_label}\"]}" >/dev/null
         info "Added label '${size_label}'"
     fi
 }