fac19 / Week8-BFOP

https://snippetsofcode.herokuapp.com/
0 stars 2 forks source link

enforceStatus #67

Closed oliverjam closed 4 years ago

oliverjam commented 4 years ago

https://github.com/fac19/Week8-BFOP/blob/6d98624ef265c11a4fc33837178a411a690db8f4/public/query.js#L1-L9

I'm a bit confused about this change to the query function. In what scenario would you want to handle a (for example) 500 error in the .then instead of the .catch?

res.ok will be false for any status outside of the 200-99 range, which cause query to throw an error. This will cause the .catch to run wherever query was called, so you can handle the unsuccessful response.

I guess I'm just curious what your use-case was for this extra bit.

Roger-Heathcote commented 4 years ago

Partly I didn't understand that OK allowed everything in the 200 range and I didn't want it catching 201s for example, good to know I was wrong about that. Partly it was that I wanted to be able to differentiate between different potential errors thrown in my model functions e.g. no matching rows found vs not authorized e.g.

                .query("DELETE FROM examples WHERE id = ($1);", [exampleId])
                .then(result => true)
                .catch(err => {
                    const error = new Error("Delete query failed!" + err.message);
                    error.status = 400;
                    throw error;
                });
        } else {
            const error = new Error("Only owner or admin can delete this.");
            error.status = 403;
            throw error;
            return false;
oliverjam commented 4 years ago

Aha that makes sense. I'd recommend handling different kinds of errors like that in a .catch where you make the request, rather than trying to do it generically here. Since we're attaching the status property to the error object you should be able to use that to determine what type of error occurred and show a relevant message to the user.