Skip to content

Commit

Permalink
Fix parsing of error in fetch tool and when to do reauth (#3777)
Browse files Browse the repository at this point in the history
  • Loading branch information
jatgarg authored Oct 15, 2020
1 parent bf91cd7 commit eab218c
Show file tree
Hide file tree
Showing 36 changed files with 210 additions and 213 deletions.
2 changes: 1 addition & 1 deletion .github/code_owners.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ packages/tools/**:
- vladsud
- anthony-murphy

packages/utils/odsp-utils/**:
packages/utils/odsp-doclib-utils/**:
- markfields
- curtisman
packages/utils/telemtry-utils/**:
Expand Down
2 changes: 1 addition & 1 deletion docs/PACKAGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ The dependencies between layers are enforced by the layer-check command._

| Packages | Layer Dependencies |
| --- | --- |
| - [@fluidframework/odsp-utils](/packages/utils/odsp-utils) |   |
| - [@fluidframework/odsp-doclib-utils](/packages/utils/odsp-doclib-utils) |   |

### Tool-Utils

Expand Down
1 change: 1 addition & 0 deletions packages/drivers/odsp-driver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"@fluidframework/driver-definitions": "^0.28.0",
"@fluidframework/driver-utils": "^0.28.0",
"@fluidframework/gitresources": "^0.1014.0-0",
"@fluidframework/odsp-doclib-utils": "0.28.0",
"@fluidframework/protocol-base": "^0.1014.0-0",
"@fluidframework/protocol-definitions": "^0.1014.0-0",
"@fluidframework/telemetry-utils": "^0.28.0",
Expand Down
9 changes: 3 additions & 6 deletions packages/drivers/odsp-driver/src/createFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
*/

import { Uint8ArrayToString } from "@fluidframework/common-utils";
import { getGitType } from "@fluidframework/protocol-base";
import { getDocAttributesFromProtocolSummary } from "@fluidframework/driver-utils";
import { fetchIncorrectResponse, invalidFileNameStatusCode } from "@fluidframework/odsp-doclib-utils";
import { getGitType } from "@fluidframework/protocol-base";
import { SummaryType, ISummaryTree, ISummaryBlob } from "@fluidframework/protocol-definitions";
import { ITelemetryLogger } from "@fluidframework/common-definitions";
import { PerformanceEvent } from "@fluidframework/telemetry-utils";
Expand All @@ -27,11 +28,7 @@ import {
} from "./odspUtils";
import { createOdspUrl } from "./createOdspUrl";
import { getApiRoot } from "./odspUrlHelper";
import {
throwOdspNetworkError,
invalidFileNameStatusCode,
fetchIncorrectResponse,
} from "./odspError";
import { throwOdspNetworkError } from "./odspError";
import { TokenFetchOptions } from "./tokenFetch";
import { OdspDriverUrlResolver } from "./odspDriverUrlResolver";

Expand Down
2 changes: 1 addition & 1 deletion packages/drivers/odsp-driver/src/fetchWithRetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { ITelemetryLogger, ITelemetryProperties } from "@fluidframework/common-definitions";
import { fetchFailureStatusCode, offlineFetchFailureStatusCode } from "./odspError";
import { fetchFailureStatusCode, offlineFetchFailureStatusCode } from "@fluidframework/odsp-doclib-utils";

/**
* returns a promise that resolves after timeMs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ITelemetryLogger } from "@fluidframework/common-definitions";
import { TelemetryNullLogger } from "@fluidframework/common-utils";
import { DocumentDeltaConnection } from "@fluidframework/driver-base";
import { IDocumentDeltaConnection, DriverError } from "@fluidframework/driver-definitions";
import { OdspError } from "@fluidframework/odsp-doclib-utils";
import {
IClient,
IConnect,
Expand All @@ -19,7 +20,7 @@ import {
import uuid from "uuid/v4";
import { IOdspSocketError } from "./contracts";
import { debug } from "./debug";
import { errorObjectFromSocketError, OdspError } from "./odspError";
import { errorObjectFromSocketError } from "./odspError";

const protocolVersions = ["^0.4.0", "^0.3.0", "^0.2.0", "^0.1.0"];

Expand Down
115 changes: 1 addition & 114 deletions packages/drivers/odsp-driver/src/odspError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,123 +3,10 @@
* Licensed under the MIT License.
*/

import {
NetworkErrorBasic,
GenericNetworkError,
NonRetryableError,
AuthorizationError,
isOnline,
createGenericNetworkError,
OnlineStatus,
} from "@fluidframework/driver-utils";
import {
DriverError,
DriverErrorType,
} from "@fluidframework/driver-definitions";
import { createOdspNetworkError } from "@fluidframework/odsp-doclib-utils";
import { IOdspSocketError } from "./contracts";
import { parseAuthErrorClaims } from "./parseAuthErrorClaims";

export const offlineFetchFailureStatusCode: number = 709;
export const fetchFailureStatusCode: number = 710;
// Status code for invalid file name error in odsp driver.
export const invalidFileNameStatusCode: number = 711;
// no response, or can't parse response
export const fetchIncorrectResponse = 712;

export enum OdspErrorType {
/**
* Storage is out of space
*/
outOfStorageError = "outOfStorageError",

/**
* Invalid file name (at creation of the file)
*/
invalidFileNameError = "invalidFileNameError",

/**
* Snapshot is too big. Host application specified limit for snapshot size, and snapshot was bigger
* that that limit, thus request failed. Hosting application is expected to have fall-back behavior for
* such case.
*/
snapshotTooBig = "snapshotTooBig",

/*
* SPO admin toggle: fluid service is not enabled.
*/
fluidNotEnabled = "fluidNotEnabled",
}

/**
* Base interface for all errors and warnings
*/
export interface IOdspError {
readonly errorType: OdspErrorType;
readonly message: string;
canRetry: boolean;
online?: string;
}

export type OdspError =
| DriverError
| IOdspError;

export function createOdspNetworkError(
errorMessage: string,
statusCode?: number,
retryAfterSeconds?: number,
claims?: string,
): OdspError {
let error: OdspError;

switch (statusCode) {
case 400:
error = new GenericNetworkError(errorMessage, false, statusCode);
break;
case 401:
case 403:
error = new AuthorizationError(errorMessage, claims);
break;
case 404:
error = new NetworkErrorBasic(errorMessage, DriverErrorType.fileNotFoundOrAccessDeniedError, false);
break;
case 406:
error = new NetworkErrorBasic(errorMessage, DriverErrorType.unsupportedClientProtocolVersion, false);
break;
case 413:
error = new NonRetryableError(errorMessage, OdspErrorType.snapshotTooBig);
break;
case 414:
case invalidFileNameStatusCode:
error = new NonRetryableError(errorMessage, OdspErrorType.invalidFileNameError);
break;
case 500:
error = new GenericNetworkError(errorMessage, true);
break;
case 501:
error = new NonRetryableError(errorMessage, OdspErrorType.fluidNotEnabled);
break;
case 507:
error = new NonRetryableError(errorMessage, OdspErrorType.outOfStorageError);
break;
case offlineFetchFailureStatusCode:
error = new NetworkErrorBasic(errorMessage, DriverErrorType.offlineError, true);
break;
case fetchFailureStatusCode:
error = new NetworkErrorBasic(errorMessage, DriverErrorType.fetchFailure, true);
break;
case fetchIncorrectResponse:
error = new NetworkErrorBasic(errorMessage, DriverErrorType.incorrectServerResponse, false);
break;
default:
error = createGenericNetworkError(errorMessage, true, retryAfterSeconds, statusCode);
}

error.online = OnlineStatus[isOnline()];

return error;
}

/**
* Throws network error - an object with a bunch of network related properties
*/
Expand Down
18 changes: 7 additions & 11 deletions packages/drivers/odsp-driver/src/odspUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* Licensed under the MIT License.
*/

import { DriverErrorType } from "@fluidframework/driver-definitions";
import { isOnline, OnlineStatus } from "@fluidframework/driver-utils";
import {
OnlineStatus, isOnline,
} from "@fluidframework/driver-utils";
import {
DriverErrorType,
} from "@fluidframework/driver-definitions";
fetchIncorrectResponse,
offlineFetchFailureStatusCode,
fetchFailureStatusCode,
} from "@fluidframework/odsp-doclib-utils";
import {
default as fetch,
RequestInfo as FetchRequestInfo,
Expand All @@ -17,12 +18,7 @@ import {
} from "node-fetch";
import sha from "sha.js";
import { debug } from "./debug";
import {
offlineFetchFailureStatusCode,
fetchFailureStatusCode,
fetchIncorrectResponse,
throwOdspNetworkError,
} from "./odspError";
import { throwOdspNetworkError } from "./odspError";
import { TokenFetchOptions } from "./tokenFetch";

/** Parse the given url and return the origin (host name) */
Expand Down
11 changes: 4 additions & 7 deletions packages/drivers/odsp-driver/src/test/odspError.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,15 @@

import { strict as assert } from "assert";
import { DriverErrorType } from "@fluidframework/driver-definitions";
import { IOdspSocketError } from "../contracts";
import {
getWithRetryForTokenRefresh,
} from "../odspUtils";
import {
createOdspNetworkError,
errorObjectFromSocketError,
fetchIncorrectResponse,
throwOdspNetworkError,
invalidFileNameStatusCode,
OdspError,
} from "../odspError";
} from "@fluidframework/odsp-doclib-utils";
import { IOdspSocketError } from "../contracts";
import { getWithRetryForTokenRefresh } from "../odspUtils";
import { errorObjectFromSocketError, throwOdspNetworkError } from "../odspError";

describe("Odsp Error", () => {
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
Expand Down
2 changes: 1 addition & 1 deletion packages/test/end-to-end-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"@fluidframework/matrix": "^0.28.0",
"@fluidframework/merge-tree": "^0.28.0",
"@fluidframework/mocha-test-setup": "^0.28.0",
"@fluidframework/odsp-driver": "^0.28.0",
"@fluidframework/odsp-doclib-utils": "^0.28.0",
"@fluidframework/ordered-collection": "^0.28.0",
"@fluidframework/protocol-definitions": "^0.1014.0-0",
"@fluidframework/register-collection": "^0.28.0",
Expand Down
10 changes: 5 additions & 5 deletions packages/test/end-to-end-tests/src/test/error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,26 @@
*/

import { strict as assert } from "assert";
import { IRequest } from "@fluidframework/core-interfaces";
import {
ContainerErrorType,
} from "@fluidframework/container-definitions";
import { Container, Loader } from "@fluidframework/container-loader";
import { CreateContainerError } from "@fluidframework/container-utils";
import { IRequest } from "@fluidframework/core-interfaces";
import {
IFluidResolvedUrl,
IDocumentServiceFactory,
DriverErrorType,
IThrottlingWarning,
} from "@fluidframework/driver-definitions";
import { createWriteError } from "@fluidframework/driver-utils";
import { CustomErrorWithProps } from "@fluidframework/telemetry-utils";
import { CreateContainerError } from "@fluidframework/container-utils";
import { LocalDocumentServiceFactory, LocalResolver } from "@fluidframework/local-driver";
import {
createOdspNetworkError,
invalidFileNameStatusCode,
OdspErrorType,
} from "@fluidframework/odsp-driver";
import { LocalDocumentServiceFactory, LocalResolver } from "@fluidframework/local-driver";
} from "@fluidframework/odsp-doclib-utils";
import { CustomErrorWithProps } from "@fluidframework/telemetry-utils";
import { ILocalDeltaConnectionServer, LocalDeltaConnectionServer } from "@fluidframework/server-local-server";
import { LocalCodeLoader } from "@fluidframework/test-utils";

Expand Down
2 changes: 1 addition & 1 deletion packages/tools/fetch-tool/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
"@fluidframework/datastore": "^0.28.0",
"@fluidframework/driver-definitions": "^0.28.0",
"@fluidframework/driver-utils": "^0.28.0",
"@fluidframework/odsp-doclib-utils": "^0.28.0",
"@fluidframework/odsp-driver": "^0.28.0",
"@fluidframework/odsp-urlresolver": "^0.28.0",
"@fluidframework/odsp-utils": "^0.28.0",
"@fluidframework/protocol-definitions": "^0.1014.0-0",
"@fluidframework/routerlicious-driver": "^0.28.0",
"@fluidframework/routerlicious-urlresolver": "^0.28.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/tools/fetch-tool/src/fluidFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import fs from "fs";
import util from "util";
import { isSharepointURL, IOdspDriveItem } from "@fluidframework/odsp-utils";
import { isSharepointURL, IOdspDriveItem } from "@fluidframework/odsp-doclib-utils";
import { paramSaveDir, paramURL, parseArguments } from "./fluidFetchArgs";
import { connectionInfo, fluidFetchInit } from "./fluidFetchInit";
import { fluidFetchMessages } from "./fluidFetchMessages";
Expand Down
2 changes: 1 addition & 1 deletion packages/tools/fetch-tool/src/fluidFetchInit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import child_process from "child_process";
import { IFluidResolvedUrl, IResolvedUrl, IUrlResolver } from "@fluidframework/driver-definitions";
import { configurableUrlResolver } from "@fluidframework/driver-utils";
import { FluidAppOdspUrlResolver } from "@fluid-internal/fluidapp-odsp-urlresolver";
import { IClientConfig, IOdspAuthRequestInfo } from "@fluidframework/odsp-doclib-utils";
import * as odsp from "@fluidframework/odsp-driver";
import { OdspUrlResolver } from "@fluidframework/odsp-urlresolver";
import { IClientConfig, IOdspAuthRequestInfo } from "@fluidframework/odsp-utils";
import * as r11s from "@fluidframework/routerlicious-driver";
import { RouterliciousUrlResolver } from "@fluidframework/routerlicious-urlresolver";
import { getMicrosoftConfiguration } from "@fluidframework/tool-utils";
Expand Down
19 changes: 6 additions & 13 deletions packages/tools/fetch-tool/src/fluidFetchSharePoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License.
*/

import { DriverErrorType } from "@fluidframework/driver-definitions";
import {
getChildrenByDriveItem,
getDriveItemByServerRelativePath,
Expand All @@ -11,7 +12,7 @@ import {
IOdspDriveItem,
getOdspRefreshTokenFn,
IOdspAuthRequestInfo,
} from "@fluidframework/odsp-utils";
} from "@fluidframework/odsp-doclib-utils";
import {
getMicrosoftConfiguration,
OdspTokenManager,
Expand Down Expand Up @@ -56,19 +57,11 @@ export async function resolveWrapper<T>(
}
return result;
} catch (e) {
if (e.requestResultError) {
const parsedBody = JSON.parse(e.requestResult.data);
if (parsedBody.error === "invalid_grant"
&& parsedBody.suberror === "consent_required"
&& !forceTokenReauth
) {
// Re-auth
return resolveWrapper<T>(callback, server, clientConfig, true);
}
const responseMsg = JSON.stringify(parsedBody.error, undefined, 2);
return Promise.reject(`Fail to connect to ODSP server\nError Response:\n${responseMsg}`);
if (e.errorType === DriverErrorType.authorizationError && !forceTokenReauth) {
// Re-auth
return resolveWrapper<T>(callback, server, clientConfig, true, forToken);
}
throw e;
return Promise.reject(`Fail to connect to ODSP server\nError Response:\n${e}`);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/tools/webpack-fluid-loader/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
"@fluidframework/driver-definitions": "^0.28.0",
"@fluidframework/driver-utils": "^0.28.0",
"@fluidframework/local-driver": "^0.28.0",
"@fluidframework/odsp-doclib-utils": "^0.28.0",
"@fluidframework/odsp-driver": "^0.28.0",
"@fluidframework/odsp-utils": "^0.28.0",
"@fluidframework/protocol-definitions": "^0.1014.0-0",
"@fluidframework/routerlicious-driver": "^0.28.0",
"@fluidframework/runtime-utils": "^0.28.0",
Expand Down
6 changes: 3 additions & 3 deletions packages/tools/webpack-fluid-loader/src/odspUrlResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
* Licensed under the MIT License.
*/

import { IUrlResolver, IResolvedUrl } from "@fluidframework/driver-definitions";
import { IRequest } from "@fluidframework/core-interfaces";
import { OdspDriverUrlResolver, createOdspUrl } from "@fluidframework/odsp-driver";
import { IUrlResolver, IResolvedUrl } from "@fluidframework/driver-definitions";
import {
IOdspAuthRequestInfo,
getDriveItemByRootFileName,
} from "@fluidframework/odsp-utils";
} from "@fluidframework/odsp-doclib-utils";
import { OdspDriverUrlResolver, createOdspUrl } from "@fluidframework/odsp-driver";

export class OdspUrlResolver implements IUrlResolver {
private readonly driverUrlResolver = new OdspDriverUrlResolver();
Expand Down
Loading

0 comments on commit eab218c

Please sign in to comment.