Skip to content

Conversation

@malkovitc
Copy link

Summary

Preserve default description for all 21 built-in HTTP exceptions when using options object with cause.

Problem

When passing an options object with cause to built-in HTTP exceptions, the default description was lost:

// Before fix: error field would be undefined
throw new BadRequestException('message', { cause: new Error() });
// Response: { message: 'message', error: undefined, statusCode: 400 }

// After fix: error field preserves default
// Response: { message: 'message', error: 'Bad Request', statusCode: 400 }

Solution

Added destructuring default values for description in each exception constructor:

// Before
const { description, httpExceptionOptions } =
  HttpException.extractDescriptionAndOptionsFrom(descriptionOrOptions);

// After  
const { description = 'Bad Request', httpExceptionOptions } =
  HttpException.extractDescriptionAndOptionsFrom(descriptionOrOptions);

Test plan

  • Added test that verifies all 21 exceptions preserve their default description
  • All existing HttpException tests pass

Related

Closes #15961

…n using options

When passing an options object with `cause` to built-in HTTP exceptions,
the default description was lost because `extractDescriptionAndOptionsFrom`
returns `undefined` for the description property when options object is passed.

Fixed by adding destructuring default values for the description in each
exception constructor, matching the default value from the parameter.

For example:
```typescript
// Before: description would be undefined
const { description, httpExceptionOptions } = ...

// After: description defaults to 'Bad Request' if undefined
const { description = 'Bad Request', httpExceptionOptions } = ...
```

Applied fix to all 21 built-in HTTP exceptions:
- BadGatewayException
- BadRequestException
- ConflictException
- ForbiddenException
- GatewayTimeoutException
- GoneException
- HttpVersionNotSupportedException
- ImATeapotException
- InternalServerErrorException
- MethodNotAllowedException
- MisdirectedException
- NotAcceptableException
- NotFoundException
- NotImplementedException
- PayloadTooLargeException
- PreconditionFailedException
- RequestTimeoutException
- ServiceUnavailableException
- UnauthorizedException
- UnprocessableEntityException
- UnsupportedMediaTypeException

Closes nestjs#15961
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8a135727-6ce0-4aba-a7ae-912360797e37

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 88.939%

Totals Coverage Status
Change from base Build f6791497-0812-4887-9ce0-74760514d05b: 0.0%
Covered Lines: 7325
Relevant Lines: 8236

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

blocked as it introduces a minor breaking change

@malkovitc
Copy link
Author

Got it, thanks for the feedback.

Would this be acceptable for the next major version (v12)? I can keep this PR open and rebase when the time comes, or close it and open a fresh one closer to the v12 release - whatever works best for you.

Alternatively, if there's a different approach you'd prefer that could be merged sooner, I'm happy to explore that as well.

@kamilmysliwiec
Copy link
Member

yes it will! we can keep it open, no need to create another one

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Http exceptions are loosing default description if cause is provided

3 participants