fix: add extra error logging to auth response errors #6140
Conversation
🦋 Changeset detectedLatest commit: a323fbe The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
petebacondarwin
force-pushed
the
auth-logging
branch
from
de7e2c1
to
3f8f9b4
Compare
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-wrangler-6140You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6140/npm-package-wrangler-6140Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-wrangler-6140 dev path/to/script.jsAdditional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-create-cloudflare-6140 --no-auto-updatenpm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-cloudflare-kv-asset-handler-6140npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-miniflare-6140npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-cloudflare-pages-shared-6140npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9681349168/npm-package-cloudflare-vitest-pool-workers-6140Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
| @@ -1249,3 +1252,27 @@ async function fetchAuthToken(body: URLSearchParams) { | |||
| headers, | |||
| }); | |||
| } | |||
|
|
|||
| async function getJSONFromResponse(response: Response) { | |||
| const text = await response.text(); | |||
There was a problem hiding this comment.
Very minor, but if you passed in the text string to this function, it would reduce its responsibilities:
async function getJSONFromText<T>(text: string): T {
try ...
}
...
const json = getJSONFromText<TokenResponse>(await response.text());
There was a problem hiding this comment.
RIght, but then we would have to extract the text on each of the three calls, which seems a bit repetitive; but more importantly, the logs grab the status from the response so that would need to be passed in too.
There was a problem hiding this comment.
Also, we might also want to use the status code as part of the analysis of what went wrong with the response (e.g. I think that the Bot Management failure would have a 403 status).
|
We're going to hold off landing this given our limited deployment window this week. But if you are experiencing issues as linked in the description above, you could try this version of Wrangler via: Or perhaps installing it into your project (and using it as normal) via: |
petebacondarwin
force-pushed
the
auth-logging
branch
from
3f8f9b4
to
a323fbe
Compare
What this PR solves / how to test
May help diagnosing issues like #3672, #5542 and https://jira.cfdata.org/browse/CUSTESC-42645
Author has addressed the following