Throw errors from auth factory functions instead of swallowing #155
Conversation
cuzzlor
force-pushed
the
change-http-on-auth-factory-error-throw
branch
from
ab9f362 to
503bfa2
Compare
mderriey
left a comment
mderriey
left a comment
There was a problem hiding this comment.
Makes sense to me, thanks Sam!
| authHeaders = authFactory ? await authFactory(requestContext as TContext) : {} | ||
| } catch (error) { | ||
| logger?.error('Authentication via authFactory failed', { error }) | ||
| throw error |
There was a problem hiding this comment.
Will this cause a 401/403 type HTTP code?
There was a problem hiding this comment.
No - the point of my change is to prevent running the request if the auth factory function fails.
Previously the request would run anyway, without the result of the auth factory, and you'd end up with a confusing failed request which should never have started after the swallowed-and-logged auth factory error.
Essentially making for confusing AF behaviour getting an unauthorised response back because your auth factory failed, not getting your auth factory function error.
If your auth factory optionally adds auth headers, that's fine, just write the auth factory to do so.
Problem
The prior behaviour of the http module was to log but continue when calling the provided
authFactoryfunction.This results in some unexpected behaviour, since ignoring the failure would in the majority of cases cause the http request to fail, but it would be unclear to the caller clear why the http module is executing the request without the expected auth headers attached.
At least for us, this behaviour makes things more confusing, we expect any auth factory function failure to be the final stop in our code.
Proposed change
Since you would expect the
authFactoryfunction to run or fail if you provide one, change the behaviour to throw any error from theauthFactoryfunction after logging it.Also
Address audit errors by updating all packages, including running the latest ts-toolkit init and adding 2 lint ignores for
any