Skip to content

Add public FunctionsErrorCode type #6344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 21, 2022
Merged

Add public FunctionsErrorCode type #6344

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d19d314
Add public FunctionsErrorCode type
hsubox76 Jun 8, 2022
1429f81
Add changeset
hsubox76 Jun 8, 2022
87f84c2
Update tests
hsubox76 Jun 8, 2022
bad6ec5
Update functions-compat tests
hsubox76 Jun 8, 2022
99a66ee
formatting pass
hsubox76 Jun 8, 2022
7664c1e
Change wording
hsubox76 Jun 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tiny-donuts-draw.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/functions': patch
---

Update public `FunctionsErrorCode` type to include "functions/" prefix.
5 changes: 4 additions & 1 deletion common/api-review/functions.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export interface FunctionsError extends FirebaseError {
}

// @public
export type FunctionsErrorCode = 'ok' | 'cancelled' | 'unknown' | 'invalid-argument' | 'deadline-exceeded' | 'not-found' | 'already-exists' | 'permission-denied' | 'resource-exhausted' | 'failed-precondition' | 'aborted' | 'out-of-range' | 'unimplemented' | 'internal' | 'unavailable' | 'data-loss' | 'unauthenticated';
export type FunctionsErrorCode = `functions/${FunctionsErrorCodeCore}`;

// @public
export type FunctionsErrorCodeCore = 'ok' | 'cancelled' | 'unknown' | 'invalid-argument' | 'deadline-exceeded' | 'not-found' | 'already-exists' | 'permission-denied' | 'resource-exhausted' | 'failed-precondition' | 'aborted' | 'out-of-range' | 'unimplemented' | 'internal' | 'unavailable' | 'data-loss' | 'unauthenticated';

// @public
export function getFunctions(app?: FirebaseApp, regionOrCustomDomain?: string): Functions;
Expand Down
24 changes: 15 additions & 9 deletions packages/functions-compat/src/callable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ async function expectError(
await promise;
} catch (e) {
failed = true;
// Errors coming from callable functions usually have the functions
// code in the message since it's thrown inside functions.
expect(e.code).to.match(new RegExp(`functions.*/${code}`));
expect(e.code).to.equal(code);
expect(e.message).to.equal(message);
expect(e.details).to.deep.equal(details);
}
Expand Down Expand Up @@ -106,25 +104,29 @@ describe('Firebase Functions > Call', () => {
it('missing result', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('missingResultTest');
await expectError(func(), 'internal', 'Response is missing data field.');
await expectError(
func(),
'functions/internal',
'Response is missing data field.'
);
});

it('unhandled error', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('unhandledErrorTest');
await expectError(func(), 'internal', 'internal');
await expectError(func(), 'functions/internal', 'internal');
});

it('unknown error', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('unknownErrorTest');
await expectError(func(), 'internal', 'internal');
await expectError(func(), 'functions/internal', 'internal');
});

it('explicit error', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('explicitErrorTest');
await expectError(func(), 'out-of-range', 'explicit nope', {
await expectError(func(), 'functions/out-of-range', 'explicit nope', {
start: 10,
end: 20,
long: 30
Expand All @@ -134,12 +136,16 @@ describe('Firebase Functions > Call', () => {
it('http error', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('httpErrorTest');
await expectError(func(), 'invalid-argument', 'invalid-argument');
await expectError(func(), 'functions/invalid-argument', 'invalid-argument');
});

it('timeout', async () => {
const functions = createTestService(app, region);
const func = functions.httpsCallable('timeoutTest', { timeout: 10 });
await expectError(func(), 'deadline-exceeded', 'deadline-exceeded');
await expectError(
func(),
'functions/deadline-exceeded',
'deadline-exceeded'
);
});
});
4 changes: 2 additions & 2 deletions packages/functions/src/callable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { expect } from 'chai';
import * as sinon from 'sinon';
import { FirebaseApp } from '@firebase/app';
import { FunctionsErrorCode } from './public-types';
import { FunctionsErrorCodeCore } from './public-types';
import {
Provider,
ComponentContainer,
Expand All @@ -44,7 +44,7 @@ export const TEST_PROJECT = require('../../../config/project.json');
// https://github.com/chaijs/chai/issues/608
async function expectError(
promise: Promise<any>,
code: FunctionsErrorCode,
code: FunctionsErrorCodeCore,
message: string,
details?: any
): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion packages/functions/src/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { FunctionsErrorCode } from './public-types';
import { FunctionsErrorCodeCore as FunctionsErrorCode } from './public-types';
import { decode } from './serializer';
import { HttpResponseBody } from './service';
import { FirebaseError } from '@firebase/util';
Expand Down
1 change: 1 addition & 0 deletions packages/functions/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@
import { registerFunctions } from './config';

export * from './api';
export * from './public-types';

registerFunctions(fetch.bind(self));
43 changes: 25 additions & 18 deletions packages/functions/src/public-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,30 @@ export interface Functions {
customDomain: string | null;
}

/**
* Functions error code string before adding "functions/" product prefix.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it sound like the strings come before the prefix -- is that right?

Also, why specifically "product prefix?" Would that mean something like "functions/firestore?"

If we have a staged version of this, that could help me grok it I think.

Copy link
Contributor Author

@hsubox76 hsubox76 Jun 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Staging it won't add anything that isn't here, unfortunately. Users were reporting that in v8, error codes were coming back as, for example, "deadline-exceeded" (one of the possible strings here) and in v9 they are coming back (more correctly) as "functions/deadline-exceeded" (since this is the functions package). We try to put a prefix with the SDK package name and a "/" before every error thrown by that SDK package. So Firestore should be throwing errors with code "firestore/something", auth should be throwing errors with code "auth/something", etc.

Lines 99-132 below have the main documentation for this code since that's the actual error code (with the prefix added) that users will see.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

So yes, if this is how it works, I'd expect something more like "* Functions error code string following the "functions/" product prefix."

or maybe "* Functions error code string appended after "functions/" product prefix."

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. I've changed it to "appended after".

* See {@link FunctionsErrorCode} for full documentation of codes.
* @public
*/
export type FunctionsErrorCodeCore =
| 'ok'
| 'cancelled'
| 'unknown'
| 'invalid-argument'
| 'deadline-exceeded'
| 'not-found'
| 'already-exists'
| 'permission-denied'
| 'resource-exhausted'
| 'failed-precondition'
| 'aborted'
| 'out-of-range'
| 'unimplemented'
| 'internal'
| 'unavailable'
| 'data-loss'
| 'unauthenticated';

/**
* The set of Firebase Functions status codes. The codes are the same at the
* ones exposed by gRPC here:
Expand Down Expand Up @@ -112,24 +136,7 @@ export interface Functions {
* credentials for the operation.
* @public
*/
export type FunctionsErrorCode =
| 'ok'
| 'cancelled'
| 'unknown'
| 'invalid-argument'
| 'deadline-exceeded'
| 'not-found'
| 'already-exists'
| 'permission-denied'
| 'resource-exhausted'
| 'failed-precondition'
| 'aborted'
| 'out-of-range'
| 'unimplemented'
| 'internal'
| 'unavailable'
| 'data-loss'
| 'unauthenticated';
export type FunctionsErrorCode = `functions/${FunctionsErrorCodeCore}`;

/**
* An error returned by the Firebase Functions client SDK.
Expand Down