FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

refactor errors #214

Manually merged
Johann150 merged 5 commits from errors into main 2022-10-28 17:11:57 +00:00
Conversation 2 Commits 5 Files changed 145 +942 -2446
Johann150 commented 2022-10-26 21:23:03 +00:00
Owner
Copy link
  • centrally collect all error definitions and somewhat unify them
  • add descriptive message and proper HTTP status code for each error
  • refactor OpenAPI documentation generation with regard to errors
- [x] centrally collect all error definitions and somewhat unify them - [x] add descriptive message and proper HTTP status code for each error - [x] refactor OpenAPI documentation generation with regard to errors
Johann150 added 1 commit 2022-10-26 21:23:03 +00:00 
This refactors the API handler to not use default exports, be async
instead of constructing a promise and modify how errors are returned.
Johann150 commented 2022-10-26 21:24:38 +00:00
Author
Owner
Copy link

Even though this is WIP, a review of the existing commits would be welcome. I had a second look at them but especially the latest one is quite large.

Even though this is WIP, a review of the existing commits would be welcome. I had a second look at them but especially the latest one is quite large.
Johann150 added this to the (deleted) project 2022-10-26 21:24:46 +00:00
Johann150 force-pushed errors from 5736a5cc41 to fb76843c19 2022-10-27 20:44:18 +00:00 Compare
Johann150 commented 2022-10-27 20:50:20 +00:00
Author
Owner
Copy link

This also improves the generated OpenAPI specification insofar as it now only generates the errors that can really happen on an endpoint: Not showing the 418 error on all endpoints, not showing authentication errors on endpoints that do not require authentication, etc.

I also noticed that although there was the possibility to specify a specialized error return value, this was not being properly understood while generating the documentation and instead documenting all those errors as 400 errors. This has of course also been fixed.

This also improves the generated OpenAPI specification insofar as it now only generates the errors that can really happen on an endpoint: Not showing the 418 error on all endpoints, not showing authentication errors on endpoints that do not require authentication, etc. I also noticed that although there was the possibility to specify a specialized error return value, this was not being properly understood while generating the documentation and instead documenting all those errors as 400 errors. This has of course also been fixed.
Johann150 changed title from WIP: refactor errors to refactor errors 2022-10-27 20:50:26 +00:00
norm approved these changes 2022-10-27 22:45:44 +00:00
norm left a comment
Owner
Copy link

As far as I can tell, everything looks good. Seems to be also working on my local dev environment.

As far as I can tell, everything looks good. Seems to be also working on my local dev environment.
norm reviewed 2022-10-27 22:47:39 +00:00
packages/backend/src/server/api/endpoints/i/update-email.ts
@ -34,1 +22,3 @@
},
// FIXME: refactor to remove both of these errors?
// the password should not be passed as it is not compatible with using OAuth
errors: ['ACCESS_DENIED', 'INTERNAL_ERROR'],
norm commented 2022-10-27 22:47:39 +00:00
Owner
Copy link

What's the reasoning behind replacing "Unavailable email address" with INTERNAL_ERROR?

What's the reasoning behind replacing "Unavailable email address" with `INTERNAL_ERROR`?
Johann150 commented 2022-10-28 06:07:41 +00:00
Author
Owner
Copy link

Probably not a great idea to give away users email addresses, so didn't want to make that too obvious. IMHO we should think about removing it all together.

Probably not a great idea to give away users email addresses, so didn't want to make that too obvious. IMHO we should think about removing it all together.
norm marked this conversation as resolved
Johann150 added 1 commit 2022-10-28 14:58:09 +00:00
fix some lints
Some checks failed
ci/woodpecker/push/lint-client Pipeline was successful
Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
Details
ci/woodpecker/push/lint-backend Pipeline was successful
Details
ci/woodpecker/push/build Pipeline was successful
Details
ci/woodpecker/push/test Pipeline was successful
Details
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
Details
ci/woodpecker/pr/lint-backend Pipeline failed
Details
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/lint-client Pipeline failed
Details
ci/woodpecker/pr/test Pipeline failed
Details
735b9ab502
Johann150 manually merged commit 253bffd974 into main 2022-10-28 17:11:57 +00:00
Johann150 deleted branch errors 2022-10-28 17:12:11 +00:00
Johann150 added the
upkeep
 label 2022-12-23 10:25:53 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
norm
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No labels
feature
fix
upkeep
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#214
No description provided.
Delete branch "errors"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?