Skip to content

Commit 7cc2b9c

Browse files
authored
Added ability to retry unauthorized requests (#8351)
1 parent ebbb191 commit 7cc2b9c

File tree

6 files changed

+129
-11
lines changed

6 files changed

+129
-11
lines changed

common/api-review/data-connect.api.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,5 +295,8 @@ export interface TransportOptions {
295295
sslEnabled?: boolean;
296296
}
297297

298+
// @public (undocumented)
299+
export function validateDCOptions(dcOptions: ConnectorConfig): boolean;
300+
298301

299302
```

packages/data-connect/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# @firebase/data-connect
2+
## UNRELEASED
3+
* Updated reporting to use @firebase/data-connect instead of @firebase/connect
4+
* Added functionality to retry queries and mutations if the server responds with UNAUTHENTICATED.
5+

packages/data-connect/src/core/error.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ export type DataConnectErrorCode =
2323
| 'not-initialized'
2424
| 'not-supported'
2525
| 'invalid-argument'
26-
| 'partial-error';
26+
| 'partial-error'
27+
| 'unauthorized';
2728

2829
export type Code = DataConnectErrorCode;
2930

@@ -33,7 +34,8 @@ export const Code = {
3334
NOT_INITIALIZED: 'not-initialized' as DataConnectErrorCode,
3435
NOT_SUPPORTED: 'not-supported' as DataConnectErrorCode,
3536
INVALID_ARGUMENT: 'invalid-argument' as DataConnectErrorCode,
36-
PARTIAL_ERROR: 'partial-error' as DataConnectErrorCode
37+
PARTIAL_ERROR: 'partial-error' as DataConnectErrorCode,
38+
UNAUTHORIZED: 'unauthorized' as DataConnectErrorCode
3739
};
3840

3941
/** An error returned by a DataConnect operation. */

packages/data-connect/src/network/fetch.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export function dcFetch<T, U>(
4444
}
4545
const bodyStr = JSON.stringify(body);
4646
logDebug(`Making request out to ${url} with body: ${bodyStr}`);
47+
4748
return connectFetch(url, {
4849
body: bodyStr,
4950
method: 'POST',
@@ -67,6 +68,9 @@ export function dcFetch<T, U>(
6768
logError(
6869
'Error while performing request: ' + JSON.stringify(jsonResponse)
6970
);
71+
if(response.status === 401) {
72+
throw new DataConnectError(Code.UNAUTHORIZED, JSON.stringify(jsonResponse));
73+
}
7074
throw new DataConnectError(Code.OTHER, JSON.stringify(jsonResponse));
7175
}
7276
return jsonResponse;

packages/data-connect/src/network/transport/rest.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { DataConnectError, Code } from '../../core/error';
2020
import { AuthTokenProvider } from '../../core/FirebaseAuthProvider';
2121
import { logDebug } from '../../logger';
2222
import { addToken, urlBuilder } from '../../util/url';
23-
import { dcFetch } from '../fetch';
23+
import { dcFetch, initializeFetch } from '../fetch';
2424

2525
import { DataConnectTransport } from '.';
2626

@@ -34,6 +34,7 @@ export class RESTTransport implements DataConnectTransport {
3434
private _serviceName: string;
3535
private _accessToken: string | null = null;
3636
private _authInitialized = false;
37+
private _lastToken: string | null = null;
3738
constructor(
3839
options: DataConnectOptions,
3940
private apiKey?: string | undefined,
@@ -93,14 +94,14 @@ export class RESTTransport implements DataConnectTransport {
9394
this._accessToken = newToken;
9495
}
9596

96-
getWithAuth() {
97+
getWithAuth(forceToken = false) {
9798
let starterPromise: Promise<string | null> = new Promise(resolve =>
9899
resolve(this._accessToken)
99100
);
100101
if (!this._authInitialized) {
101102
if (this.authProvider) {
102103
starterPromise = this.authProvider
103-
.getToken(/*forceToken=*/ false)
104+
.getToken(/*forceToken=*/ forceToken)
104105
.then(data => {
105106
if (!data) {
106107
return null;
@@ -115,13 +116,38 @@ export class RESTTransport implements DataConnectTransport {
115116
return starterPromise;
116117
}
117118

119+
_setLastToken(lastToken: string | null) {
120+
this._lastToken = lastToken;
121+
}
122+
123+
withRetry<T>(promiseFactory: () => Promise<{ data: T, errors: Error[]}>, retry = false) {
124+
let isNewToken = false;
125+
return this.getWithAuth(retry)
126+
.then(res => {
127+
isNewToken = this._lastToken !== res;
128+
this._lastToken = res;
129+
return res;
130+
})
131+
.then(promiseFactory)
132+
.catch(err => {
133+
// Only retry if the result is unauthorized and the last token isn't the same as the new one.
134+
if (
135+
'code' in err &&
136+
err.code === Code.UNAUTHORIZED &&
137+
!retry && isNewToken
138+
) {
139+
logDebug('Retrying due to unauthorized');
140+
return this.withRetry(promiseFactory, true);
141+
}
142+
throw err;
143+
});
144+
}
145+
118146
// TODO(mtewani): Update U to include shape of body defined in line 13.
119147
invokeQuery = <T, U = unknown>(queryName: string, body: U) => {
120148
const abortController = new AbortController();
121-
122149
// TODO(mtewani): Update to proper value
123-
const withAuth = this.getWithAuth().then(() => {
124-
return dcFetch<T, U>(
150+
const withAuth = this.withRetry(() => dcFetch<T, U>(
125151
addToken(`${this.endpointUrl}:executeQuery`, this.apiKey),
126152
{
127153
name: `projects/${this._project}/locations/${this._location}/services/${this._serviceName}/connectors/${this._connectorName}`,
@@ -130,16 +156,15 @@ export class RESTTransport implements DataConnectTransport {
130156
} as unknown as U, // TODO(mtewani): This is a patch, fix this.
131157
abortController,
132158
this._accessToken
133-
);
134-
});
159+
));
135160

136161
return {
137162
then: withAuth.then.bind(withAuth)
138163
};
139164
};
140165
invokeMutation = <T, U = unknown>(mutationName: string, body: U) => {
141166
const abortController = new AbortController();
142-
const taskResult = this.getWithAuth().then(() => {
167+
const taskResult = this.withRetry(() => {
143168
return dcFetch<T, U>(
144169
addToken(`${this.endpointUrl}:executeMutation`, this.apiKey),
145170
{
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { FirebaseAuthTokenData } from '@firebase/auth-interop-types';
2+
import {
3+
AuthTokenListener,
4+
AuthTokenProvider,
5+
DataConnectOptions,
6+
FirebaseAuthProvider
7+
} from '../../src';
8+
import { RESTTransport } from '../../src/network/transport/rest';
9+
import { initializeFetch } from '../../src/network/fetch';
10+
import { expect } from 'chai';
11+
import * as chai from 'chai';
12+
import chaiAsPromised from 'chai-as-promised';
13+
import * as sinon from 'sinon';
14+
chai.use(chaiAsPromised);
15+
const options: DataConnectOptions = {
16+
connector: 'c',
17+
location: 'l',
18+
projectId: 'p',
19+
service: 's'
20+
};
21+
const INITIAL_TOKEN = 'initial token';
22+
class FakeAuthProvider implements AuthTokenProvider {
23+
private token: string | null = INITIAL_TOKEN;
24+
addTokenChangeListener(listener: AuthTokenListener): void {}
25+
getToken(forceRefresh: boolean): Promise<FirebaseAuthTokenData | null> {
26+
if (!forceRefresh) {
27+
return Promise.resolve({ accessToken: this.token! });
28+
}
29+
return Promise.resolve({ accessToken: 'testToken' });
30+
}
31+
setToken(_token: string | null) {
32+
this.token = _token;
33+
}
34+
}
35+
const json = {
36+
message: 'unauthorized'
37+
};
38+
39+
const fakeFetchImpl = sinon.stub().returns(
40+
Promise.resolve({
41+
json: () => {
42+
return Promise.resolve(json);
43+
},
44+
status: 401
45+
} as Response)
46+
);
47+
describe('Queries', () => {
48+
afterEach(() => {
49+
fakeFetchImpl.resetHistory();
50+
});
51+
it('[QUERY] should retry auth whenever the fetcher returns with unauthorized', async () => {
52+
initializeFetch(fakeFetchImpl);
53+
const authProvider = new FakeAuthProvider();
54+
const rt = new RESTTransport(options, undefined, authProvider);
55+
await expect(
56+
rt.invokeQuery('test', null)
57+
).to.eventually.be.rejectedWith(JSON.stringify(json));
58+
expect(fakeFetchImpl.callCount).to.eq(2);
59+
});
60+
it('[MUTATION] should retry auth whenever the fetcher returns with unauthorized', async () => {
61+
initializeFetch(fakeFetchImpl);
62+
const authProvider = new FakeAuthProvider();
63+
const rt = new RESTTransport(options, undefined, authProvider);
64+
await expect(
65+
rt.invokeMutation('test', null)
66+
).to.eventually.be.rejectedWith(JSON.stringify(json));
67+
expect(fakeFetchImpl.callCount).to.eq(2);
68+
});
69+
it("should not retry auth whenever the fetcher returns with unauthorized and the token doesn't change", async () => {
70+
initializeFetch(fakeFetchImpl);
71+
const authProvider = new FakeAuthProvider();
72+
const rt = new RESTTransport(options, undefined, authProvider);
73+
rt._setLastToken('initial token');
74+
await expect(
75+
rt.invokeQuery('test', null) as Promise<any>
76+
).to.eventually.be.rejectedWith(JSON.stringify(json));
77+
expect(fakeFetchImpl.callCount).to.eq(1);
78+
});
79+
});

0 commit comments

Comments
 (0)