Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes #19835

Merged
merged 4 commits into from
Mar 12, 2024
Merged

Misc fixes #19835

merged 4 commits into from
Mar 12, 2024

Conversation

daniellockyer
Copy link
Member

No description provided.

- made fixes for the following:
  - jsdoc definitions
  - typos
  - extra parameter to function
  - missing `utf-8` to fs file read
fixes ENG-733
ref https://linear.app/tryghost/issue/ENG-733/handle-image-uploads-where-name-is-too-long

- filesystems usually have a filename length limit; ie. on macOS it is
  255 characters
- if a file is uploaded with a longer filename, we'll return a HTTP 500
- we shouldn't do this as it is user error, so we can just catch the
  error code and return BadRequest
- this implements that, and adds a breaking test
fixes TryGhost/Product#4237

- this fixes the fact that we return a HTTP 500 response when the oembed
  library receives an error, such as a 401 or 403
- includes special handling for cases where we want to return a slightly
  different error message
- also adds unit tests for @tryghost/oembed-service package
ref ENG-737
ref https://linear.app/tryghost/issue/ENG-737/http-500-errors-from-recommendations-check-endpoint

- it's still possible for `this.#externalRequest.get` to throw, like if
  DNS resolution fails
- we want to try-catch this so we don't throw from this function and
  return a HTTP 500 to the user
- instead, we can just return `undefined`, which is the fallback
- adds a breaking test too
@daniellockyer daniellockyer marked this pull request as ready for review March 12, 2024 11:31
@daniellockyer daniellockyer merged commit dea639e into main Mar 12, 2024
20 checks passed
@daniellockyer daniellockyer deleted the daniel-misc branch March 12, 2024 11:31
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.

1 participant