Skip to content

feat: Add more observability to getPreSignedUrl errors and prevent retries f…#335

Open
calvin-codecov wants to merge 3 commits into
mainfrom
cy/presigned_url_fix
Open

feat: Add more observability to getPreSignedUrl errors and prevent retries f…#335
calvin-codecov wants to merge 3 commits into
mainfrom
cy/presigned_url_fix

Conversation

@calvin-codecov
Copy link
Copy Markdown

@calvin-codecov calvin-codecov commented May 6, 2026 •
edited
Loading

…or client auth errors

Related to https://github.com/codecov/core-engineering/issues/96

According to the Sentry instances of this issue, there are instances of both "tokenless" and "token", "vite" vs "webpack", with and w/o git_service set, and across versions. The error is logged as the response for getPresignedUrl request which returns with ok === False as the server is returning a 4xx status codes.

  • I've added code to pull out body messaging from the 4xx server responses to hopefully give more information into why these requests are erroring on the server
  • Added code to prevent retrying the request multiple times if the response is 4xx response as this will have the same result each time
  • Not logging to Sentry for 401/403 for authentication errors as the problem is in user's setup
  • Ran prettier command which formatted a couple of unrelated files

@sentry
Copy link
Copy Markdown

sentry Bot commented May 6, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 85.24590% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.72%. Comparing base (78a72cb) to head (0e9bc91).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...s/bundler-plugin-core/src/utils/getPreSignedURL.ts 79.41% 7 Missing ⚠️
...dler-plugin-core/src/errors/AuthenticationError.ts 60.00% 2 Missing ⚠️
Additional details and impacted files
Components Coverage Δ
Plugin core 97.85% <85.24%> (-0.21%) ⬇️
Rollup plugin 8.42% <ø> (ø)
Vite plugin 8.42% <ø> (ø)
Webpack plugin 56.84% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link
Copy Markdown

codecov-notifications Bot commented May 7, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 85.24590% with 9 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...s/bundler-plugin-core/src/utils/getPreSignedURL.ts 79.41% 7 Missing ⚠️
...dler-plugin-core/src/errors/AuthenticationError.ts 60.00% 2 Missing ⚠️
Components Coverage Δ
Plugin core 97.85% <85.24%> (-0.21%) ⬇️
Rollup plugin 8.42% <ø> (ø)
Vite plugin 8.42% <ø> (ø)
Webpack plugin 56.84% <ø> (ø)

📢 Thoughts on this report? Let us know!

@sentry
Copy link
Copy Markdown

sentry Bot commented May 7, 2026 •
edited
Loading

Bundle Report

Changes will decrease total bundle size by 20.35kB (-0.23%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@codecov/vite-plugin-esm 1.24kB -5.15kB (-80.6%) ⬇️
@codecov/bundler-plugin-core-cjs 1.55MB 1.5kB (0.1%) ⬆️
@codecov/rollup-plugin-esm 1.3kB -5.11kB (-79.7%) ⬇️
@codecov/webpack-plugin-esm 3.45kB -5.43kB (-61.16%) ⬇️
@codecov/example-sveltekit-app-client-esm 727.67kB -2 bytes (-0.0%) ⬇️
@codecov/example-sveltekit-app-server-esm 984.06kB -1 bytes (-0.0%) ⬇️
@codecov/nextjs-webpack-plugin-esm 1.11kB -3.74kB (-77.06%) ⬇️
@codecov/astro-plugin-esm 862 bytes -2.41kB (-73.62%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: @codecov/example-astro-5-app-server-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
manifest_BSN_ztv2.mjs (New) 3.37kB 3.37kB 100.0% 🚀
manifest_BnHbmTZ8.mjs (Deleted) -3.37kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/astro-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.cts (New) 862 bytes 862 bytes 100.0% 🚀
index.mjs (Deleted) -3.27kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/sveltekit-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.mts (New) 891 bytes 891 bytes 100.0% 🚀
index.d.cts (Deleted) -891 bytes 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/example-astro-app-server-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
manifest_x0Si20gi.mjs (New) 3.34kB 3.34kB 100.0% 🚀
manifest_BBTABJzS.mjs (Deleted) -3.34kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/nextjs-webpack-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.mts (New) 1.11kB 1.11kB 100.0% 🚀
index.mjs (Deleted) -4.86kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/example-next-15-app-client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
static/IbRXJTxyKH87ghDo9AjDd/_buildManifest.js (New) 543 bytes 543 bytes 100.0% 🚀
static/IbRXJTxyKH87ghDo9AjDd/_ssgManifest.js (New) 77 bytes 77 bytes 100.0% 🚀
static/VsxTCkvmo7lpIVBKhG2u5/_buildManifest.js (Deleted) -543 bytes 0 bytes -100.0% 🗑️
static/VsxTCkvmo7lpIVBKhG2u5/_ssgManifest.js (Deleted) -77 bytes 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/rollup-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.ts (New) 1.3kB 1.3kB 100.0% 🚀
index.mjs (Deleted) -6.41kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/webpack-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.ts (New) 3.45kB 3.45kB 100.0% 🚀
index.mjs (Deleted) -8.89kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/bundler-plugin-core-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.cjs 1.5kB 1.55MB 0.1%

Files in index.cjs:

  • ./src/utils/fetchWithRetry.ts → Total Size: 1.03kB

  • ./src/utils/Output.ts → Total Size: 11.25kB

  • ./src/utils/getPreSignedURL.ts → Total Size: 5.61kB

  • ./src/errors/AuthenticationError.ts → Total Size: 102 bytes

view changes for bundle: @codecov/example-sveltekit-app-client-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
_app/immutable/chunks/entry.*.js -2 bytes 31.45kB -0.01%
view changes for bundle: @codecov/vite-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.ts (New) 1.24kB 1.24kB 100.0% 🚀
index.mjs (Deleted) -6.39kB 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/example-sveltekit-app-server-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
chunks/internal.js -1 bytes 18.48kB -0.01%
view changes for bundle: @codecov/example-next-app-client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
server/middleware-*.js -852 bytes 36 bytes -95.95%
server/middleware-*.js 852 bytes 888 bytes 2366.67% ⚠️
static/Zg4eUEyI6xP7xlI6tdQHl/_buildManifest.js (New) 224 bytes 224 bytes 100.0% 🚀
static/Zg4eUEyI6xP7xlI6tdQHl/_ssgManifest.js (New) 77 bytes 77 bytes 100.0% 🚀
static/T3uU1sdMcILEO2L4xkS3Z/_buildManifest.js (Deleted) -224 bytes 0 bytes -100.0% 🗑️
static/T3uU1sdMcILEO2L4xkS3Z/_ssgManifest.js (Deleted) -77 bytes 0 bytes -100.0% 🗑️
view changes for bundle: @codecov/solidstart-plugin-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
index.d.ts (New) 949 bytes 949 bytes 100.0% 🚀
index.d.cts (Deleted) -949 bytes 0 bytes -100.0% 🗑️

calvin-codecov
calvin-codecov commented May 7, 2026
View reviewed changes
Comment thread packages/bundler-plugin-core/src/utils/Output.ts
await safeFlushTelemetry(this.sentryClient);
// if the user gets an authentication error, something is wrong with their setup
// so we don't want to log it to sentry
if (!(error instanceof AuthenticationError)) {
Copy link
Copy Markdown
Author

@calvin-codecov calvin-codecov May 7, 2026

Choose a reason for hiding this comment

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

this block just adds this if wrapper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomasrockhu-codecov thomasrockhu-codecov thomasrockhu-codecov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@calvin-codecov @thomasrockhu-codecov