Skip to content
This repository was archived by the owner on Dec 27, 2024. It is now read-only.

Added some error handling#15

Draft
cameronbrill wants to merge 6 commits intomasterfrom
cameronbrill/error-handling
Draft

Added some error handling#15
cameronbrill wants to merge 6 commits intomasterfrom
cameronbrill/error-handling

Conversation

@cameronbrill
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Dec 30, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eightants/idemoed/g4gcbzl3m
✅ Preview: https://idemoed-git-cameronbrill-error-handling.eightants.vercel.app

@cameronbrill cameronbrill marked this pull request as draft December 30, 2020 03:21
@geooot
Copy link
Collaborator

geooot commented Jan 8, 2021

Very cool PR!

I like the error handling (it is very much needed). But I think the service functions (devpost.ts) should be able to throw new Error('error message') instead of returning a new object type with the message. I feel like in that case the "promise" of devpost projects or hackathons is not achieved, so it should reject the promise rather than accept the promise with an object that shows the empty state.

Then on the controller side (in this case our route handler) they could try catch for the thrown errors and return a error response object.

This also helps us cover our butts as it also catches other errors that aren't explicitly thrown by us.

Idk, just a thought. Either way the implementation looks great!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants