Skip to content

Commit

Permalink
fix(server): Load errorLog regardless of credential refresh result (#…
Browse files Browse the repository at this point in the history
…3103)

<!-- Describe the problem and your solution --> 
When viewing a connection we had a strange state that was possible where
there was an auth error on a connection but because it was not a
connection that could be refreshed it wasn't showing the error in the UI
once viewing the settings.

<!-- Issue ticket number and link (if applicable) -->
See
https://linear.app/nango/issue/NAN-2255/authorization-tab-for-connections-sometimes-doesnt-show-auth-errors


<!-- Testing instructions (skip if just adding/editing providers) -->
## How I tested it

I set up an auth error row in my test db on a connection that didn't
refresh (I used Unauthenticated) and confirmed that it displayed
correctly. Here's a sample insert (you'll need to tweak it to get it to
work with a connection you have set up):

```
INSERT INTO "nango"."_nango_active_logs"("id","type","action","connection_id","log_id","active","sync_id","created_at","updated_at")
VALUES
(391,E'auth',E'token_refresh',73,E'begpVdnwQLenQDHNVicf',TRUE,NULL,E'2024-11-26 16:53:33.628375+00',E'2024-11-26 16:53:33.628375+00');
```

Before this change you would see the auth error in the connections list,
but visiting the connection would not show a dot on the auth tab or show
an error on that page. After this change you'll see the dot and the
error.
  • Loading branch information
nalanj authored Dec 3, 2024
1 parent b97bcfd commit 5608eaa
Showing 1 changed file with 4 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,31 +83,18 @@ export const getConnection = asyncWrapper<GetConnection>(async (req, res) => {
onRefreshSuccess: connectionRefreshSuccessHook,
onRefreshFailed: connectionRefreshFailedHook
});
if (credentialResponse.isErr()) {
const errorLog = await errorNotificationService.auth.get(connection.id!);

// When we failed to refresh we still return a 200 because the connection is used in the UI
// Ultimately this could be a second endpoint so the UI displays faster and no confusion between error code
res.status(200).send({
data: {
errorLog,
provider: integration.provider,
connection: connectionFullToApi(connection as DBConnection),
endUser: endUserToApi(endUser)
}
});

return;
if (credentialResponse.isOk()) {
connection = credentialResponse.value;
}

connection = credentialResponse.value;
const errorLog = await errorNotificationService.auth.get(connection.id!);

res.status(200).send({
data: {
provider: integration.provider,
connection: connectionFullToApi(connection as DBConnection),
endUser: endUserToApi(endUser),
errorLog: null
errorLog
}
});
});

0 comments on commit 5608eaa

Please sign in to comment.