Skip to content

feat: add error code, normalize more errors, and exclude stack traces from 4xx errors#1061

Merged
itslenny merged 4 commits into
masterfrom
lenny/log-fixes-error-codes-and-stacks
Apr 28, 2026
Merged

feat: add error code, normalize more errors, and exclude stack traces from 4xx errors#1061
itslenny merged 4 commits into
masterfrom
lenny/log-fixes-error-codes-and-stacks

Conversation

@itslenny
Copy link
Copy Markdown
Contributor

@itslenny itslenny commented Apr 27, 2026

What kind of change does this PR introduce?

Feature

What is the current behavior?

  • logs all stack traces. 404, 400, etc are very noisy and not useful and loading all stack traces adds up to real performance impact at scale
  • error code can only be accessed via error.raw
  • several errors were missing data (error code and/or status code)

What is the new behavior?

Only log stack traces for http errors >= 500 and unknown errors

Add errorCode to logged errors to expose the error codes defined in the docs. Prior to this change these can only be accessed by digging in to error.raw

Normalized Errors

  • Missing error code. These errors had no error code
    • TUS metadata parse error = InvalidRequest
    • S3 continuation error = InvalidParameter
    • S3 invalid XML = InvalidRequest
    • S3 AJV validation errors = InvalidRequest
    • S3 stream errors = various... see: signature-v4-stream.ts
    • Iceberg errors
      • Map values from iceberg error format which differs from other storage api errors
  • Fastify Validation (map to InvalidRequest)
    • FST_ERR_VALIDATION
    • FST_ERR_CTP_INVALID_JSON_BODY
    • FST_ERR_CTP_INVALID_CONTENT_LENGTH
    • FST_ERR_CTP_INVALID_MEDIA_TYPE
    • FST_ERR_CTP_BODY_TOO_LARGE

@itslenny itslenny requested a review from a team as a code owner April 27, 2026 17:30
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 27, 2026

Coverage Report for CI Build 25052191370

Coverage increased (+0.1%) to 71.734%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: 8 uncovered changes across 4 files (39 of 47 lines covered, 82.98%).
  • 3 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
src/storage/protocols/s3/signature-v4-stream.ts 17 12 70.59%
src/http/routes/s3/index.ts 1 0 0.0%
src/storage/object.ts 1 0 0.0%
src/storage/protocols/s3/s3-handler.ts 1 0 0.0%

Coverage Regressions

3 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
src/storage/protocols/s3/signature-v4-stream.ts 2 91.25%
src/http/routes/s3/error-handler.ts 1 52.94%

Coverage Stats

Coverage Status
Relevant Lines: 9793
Covered Lines: 7429
Line Coverage: 75.86%
Relevant Branches: 5494
Covered Branches: 3537
Branch Coverage: 64.38%
Branches in Coverage %: Yes
Coverage Strength: 383.75 hits per line

💛 - Coveralls

Comment thread src/internal/errors/codes.ts Outdated
Comment thread src/internal/errors/codes.ts
Comment thread src/internal/errors/codes.ts Outdated
Copy link
Copy Markdown
Member

@ferhatelmas ferhatelmas left a comment

Choose a reason for hiding this comment

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

requesting changes to prevent merge by mistake

@itslenny itslenny force-pushed the lenny/log-fixes-error-codes-and-stacks branch from e9c8e9c to b784b38 Compare April 27, 2026 20:49
@itslenny itslenny requested a review from ferhatelmas April 27, 2026 20:49
Comment thread src/internal/errors/codes.ts Outdated
Comment thread src/internal/errors/codes.ts Outdated
Comment thread src/internal/errors/codes.ts Outdated
Comment thread src/internal/errors/codes.ts
Comment thread src/internal/errors/codes.ts
Comment thread src/internal/errors/codes.ts
@itslenny itslenny requested a review from ferhatelmas April 28, 2026 12:20
@itslenny itslenny merged commit 6ca72ed into master Apr 28, 2026
11 checks passed
@itslenny itslenny deleted the lenny/log-fixes-error-codes-and-stacks branch April 28, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants