diff --git a/actions/AuthAction.js b/actions/AuthAction.js index 57c57c3b..ad312431 100644 --- a/actions/AuthAction.js +++ b/actions/AuthAction.js @@ -12,58 +12,57 @@ const { Core } = require('@adobe/aio-sdk'); const QueryStringAddon = require('wretch/addons/queryString'); const { ImsClient } = require('./ImsClient.js'); -const { wretchRetry } = require('./Network.js'); +const wretch = require('./Network.js'); const logger = Core.Logger('AuthAction'); async function isValidToken(endpoint, clientId, token) { - return wretchRetry(`${endpoint}/ims/validate_token/v1`) - .addon(QueryStringAddon).query({ - client_id: clientId, - type: 'access_token', - }) - .headers({ - Authorization: `Bearer ${token}`, - 'Content-Type': 'application/json', - }) - .get() - .json() - .then((json) => { - return json.valid; - }) - .catch((error) => { - logger.error(error); - return false; - }); + try { + const response = await wretch(`${endpoint}/ims/validate_token/v1`) + .addon(QueryStringAddon).query({ + client_id: clientId, + type: 'access_token', + }) + .headers({ + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + }) + .get() + .json(); + return response.valid; + } catch (error) { + logger.error(error); + return false; + } } async function checkForProductContext(endpoint, clientId, org, token, productContext) { - return wretchRetry(`${endpoint}/ims/profile/v1`) - .addon(QueryStringAddon).query({ - client_id: clientId, - }) - .headers({ - Authorization: `Bearer ${token}`, - 'Content-Type': 'application/json', - }) - .get() - .json() - .then(async (json) => { - if (Array.isArray(json.projectedProductContext)) { - const filteredProductContext = json.projectedProductContext - .filter((obj) => obj.prodCtx.serviceCode === productContext); + try { + const response = await wretch(`${endpoint}/ims/profile/v1`) + .addon(QueryStringAddon).query({ + client_id: clientId, + }) + .headers({ + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + }) + .get() + .json(); - // For each entry in filteredProductContext check that - // there is at least one entry where imsOrg matches the owningEntity property - // otherwise, if no match, the user is not authorized - return filteredProductContext.some((obj) => obj.prodCtx.owningEntity === org); - } - return false; - }) - .catch((error) => { - logger.error(error); - return false; - }); + if (Array.isArray(response.projectedProductContext)) { + const filteredProductContext = response.projectedProductContext + .filter((obj) => obj.prodCtx.serviceCode === productContext); + + // For each entry in filteredProductContext check that + // there is at least one entry where imsOrg matches the owningEntity property + // otherwise, if no match, the user is not authorized + return filteredProductContext.some((obj) => obj.prodCtx.owningEntity === org); + } + return false; + } catch (error) { + logger.error(error); + return false; + } } function asAuthAction(action) { diff --git a/actions/FirefallClient.js b/actions/FirefallClient.js index ae22b39d..460c7eed 100644 --- a/actions/FirefallClient.js +++ b/actions/FirefallClient.js @@ -10,20 +10,22 @@ * governing permissions and limitations under the License. */ const { Core } = require('@adobe/aio-sdk'); -const { wretchRetry } = require('./Network.js'); +const wretch = require('./Network.js'); +const InternalError = require('./InternalError.js'); const logger = Core.Logger('FirefallAction'); -const FIREFALL_ERROR_CODES = { +const ERROR_CODES = { defaultCompletion: 'An error occurred while generating results', defaultFeedback: 'An error occurred while sending feedback', 400: "The response was filtered due to the prompt triggering Generative AI's content management policy. Please modify your prompt and retry.", + 408: "Generative AI's request timed out. Please try again.", 429: "Generative AI's Rate limit exceeded. Please wait one minute and try again.", }; -function firefallErrorMessage(defaultMessage, errorStatus) { - const errorString = FIREFALL_ERROR_CODES[errorStatus] ?? defaultMessage; - return `IS-ERROR: ${errorString} (${errorStatus}).`; +function toFirefallError(error, defaultMessage) { + const errorMessage = ERROR_CODES[error.status] ?? defaultMessage; + return new InternalError(400, `IS-ERROR: ${errorMessage} (${error.status}).`); } class FirefallClient { @@ -38,7 +40,7 @@ class FirefallClient { const startTime = Date.now(); try { - const response = await wretchRetry(`${this.endpoint}/v1/completions`) + const response = await wretch(`${this.endpoint}/v1/completions`) .headers({ 'x-gw-ims-org-id': this.org, 'x-api-key': this.apiKey, @@ -69,7 +71,7 @@ class FirefallClient { return response; } catch (error) { logger.error('Failed generating results:', error); - throw new Error(firefallErrorMessage(FIREFALL_ERROR_CODES.defaultCompletion, error.status)); + throw toFirefallError(error, ERROR_CODES.defaultCompletion); } } @@ -77,7 +79,7 @@ class FirefallClient { const startTime = Date.now(); try { - const response = await wretchRetry(`${this.endpoint}/v1/feedback`) + const response = await wretch(`${this.endpoint}/v1/feedback`) .headers({ Authorization: `Bearer ${this.accessToken}`, 'x-api-key': this.apiKey, @@ -99,7 +101,7 @@ class FirefallClient { return response; } catch (error) { logger.error('Failed sending feedback:', error); - throw new Error(firefallErrorMessage(FIREFALL_ERROR_CODES.defaultFeedback, error.status)); + throw toFirefallError(error, ERROR_CODES.defaultFeedback); } } } diff --git a/actions/FirefallClient.test.js b/actions/FirefallClient.test.js index 84f33975..16ac78d9 100644 --- a/actions/FirefallClient.test.js +++ b/actions/FirefallClient.test.js @@ -9,14 +9,9 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -import { WretchError } from 'wretch'; import { FirefallClient } from './FirefallClient.js'; -import { wretchRetry } from './Network.js'; +import wretch from './Network.js'; -// Mock the 'Network.js' module -jest.mock('./Network.js'); - -// Mock the '@adobe/aio-sdk' module jest.mock('@adobe/aio-sdk', () => ({ Core: { Logger: jest.fn().mockReturnValue({ @@ -26,44 +21,34 @@ jest.mock('@adobe/aio-sdk', () => ({ }, })); -let firefall; -let error; - -function createFirefallClient() { - const client = new FirefallClient('endpoint', 'apiKey', 'org', 'accessToken'); - return client; -} - -function createWretchError(status) { - const wretchError = new WretchError('Internal Server Error'); - wretchError.status = status; - return wretchError; -} - -beforeEach(() => { - jest.clearAllMocks(); - firefall = createFirefallClient(); - error = createWretchError(500); - wretchRetry.mockImplementation(() => ({ +jest.mock('./Network.js', () => { + const wretchMock = { headers: jest.fn().mockReturnThis(), post: jest.fn().mockReturnThis(), - json: jest.fn().mockRejectedValue(error), - })); + json: jest.fn().mockResolvedValue({}), + }; + return jest.fn().mockImplementation(() => wretchMock); }); describe('FirefallClient', () => { + const sut = new FirefallClient('endpoint', 'apiKey', 'org', 'accessToken'); + + beforeEach(() => { + jest.clearAllMocks(); + }); + test('handles 400 http status in completion method', async () => { - error.status = 400; - await expect(firefall.completion('prompt')).rejects.toThrow("IS-ERROR: The response was filtered due to the prompt triggering Generative AI's content management policy. Please modify your prompt and retry. (400)."); + wretch().json.mockRejectedValue({ status: 400 }); + await expect(sut.completion('prompt')).rejects.toThrow("IS-ERROR: The response was filtered due to the prompt triggering Generative AI's content management policy. Please modify your prompt and retry. (400)."); }); test('handles 429 http status in completion method', async () => { - error.status = 429; - await expect(firefall.completion('prompt')).rejects.toThrow("IS-ERROR: Generative AI's Rate limit exceeded. Please wait one minute and try again. (429)."); + wretch().json.mockRejectedValue({ status: 429 }); + await expect(sut.completion('prompt')).rejects.toThrow("IS-ERROR: Generative AI's Rate limit exceeded. Please wait one minute and try again. (429)."); }); test('handless any http status in the feedback method', async () => { - error.status = 500; - await expect(firefall.feedback('queryId', 'sentiment')).rejects.toThrow('An error occurred while sending feedback'); + wretch().json.mockRejectedValue({ status: 500 }); + await expect(sut.feedback('queryId', 'sentiment')).rejects.toThrow('An error occurred while sending feedback'); }); }); diff --git a/actions/GenericAction.js b/actions/GenericAction.js index 87ee474f..9390ef2d 100644 --- a/actions/GenericAction.js +++ b/actions/GenericAction.js @@ -9,22 +9,36 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ + +const InternalError = require('./InternalError.js'); + +function createResponse(status, body) { + return { + headers: { 'Content-Type': 'application/json' }, + statusCode: status, + body, + }; +} + +function createSuccessResponse(body) { + return createResponse(200, body); +} + +function createErrorResponse(status, message) { + return createResponse(status, { error: message }); +} + function asGenericAction(action) { - return async function (params) { + return async (params) => { try { - return { - headers: { 'Content-Type': 'application/json' }, - statusCode: 200, - body: await action(params), - }; + return createSuccessResponse(await action(params)); } catch (e) { - return { - headers: { 'Content-Type': 'application/json' }, - statusCode: e.status ?? 500, - body: { - error: e.message ?? 'Internal Server Error', - }, - }; + if (e instanceof InternalError) { + console.error(`Internal error: ${e.message} (${e.status})`); + return createErrorResponse(e.status, e.message); + } + console.error(`Unexpected error: ${e.message}`); + return createErrorResponse(500, e.message ?? 'Internal Server Error'); } }; } diff --git a/actions/ImsClient.js b/actions/ImsClient.js index 83d94d1c..544d693a 100644 --- a/actions/ImsClient.js +++ b/actions/ImsClient.js @@ -10,8 +10,7 @@ * governing permissions and limitations under the License. */ const FormUrlAddon = require('wretch/addons/formUrl'); -const { wretchRetry } = require('./Network.js'); -// const FormDataAddon = require('wretch/addons/formData'); +const wretch = require('./Network.js'); class ImsClient { constructor(endpoint, clientId, clientSecret, permAuthCode) { @@ -22,7 +21,7 @@ class ImsClient { } async getServiceToken() { - const json = await wretchRetry(`${this.endpoint}/ims/token/v1`) + const json = await wretch(`${this.endpoint}/ims/token/v1`) .addon(FormUrlAddon).formUrl({ client_id: this.clientId, client_secret: this.clientSecret, diff --git a/actions/InternalError.js b/actions/InternalError.js new file mode 100644 index 00000000..54a348d3 --- /dev/null +++ b/actions/InternalError.js @@ -0,0 +1,20 @@ +/* + * Copyright 2023 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +class InternalError extends Error { + constructor(status, message) { + super(message); + this.name = 'InternalError'; + this.status = status; + } +} + +module.exports = InternalError; diff --git a/actions/Network.js b/actions/Network.js index 7df0e537..02ef475d 100644 --- a/actions/Network.js +++ b/actions/Network.js @@ -9,15 +9,37 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ +const AbortAddon = require('wretch/addons/abort'); + +const { WretchError } = require('wretch'); const wretch = require('wretch'); -const { retry } = require('wretch/middlewares/retry'); +const { Core } = require('@adobe/aio-sdk'); + +const logger = Core.Logger('FirefallAction'); + +const REQUEST_TIMEOUT = 55 * 1000; + +function createWretchError(status, message) { + const error = new WretchError(); + error.status = status; + error.message = message; + return error; +} -function wretchRetry(url) { +function wretchWithOptions(url) { return wretch(url) - .middlewares([retry({ - retryOnNetworkError: true, - until: (response) => response && (response.ok || (response.status >= 400 && response.status < 500)), - })]); + .addon(AbortAddon()) + .resolve((resolver) => resolver.setTimeout(REQUEST_TIMEOUT)) + .resolve((resolver) => { + return resolver.fetchError((error) => { + if (error.name === 'AbortError') { + logger.error('Request aborted', error); + throw createWretchError(408, 'Request timed out'); + } + logger.error('Network error', error); + throw createWretchError(500, 'Network error'); + }); + }); } -module.exports = { wretchRetry }; +module.exports = wretchWithOptions; diff --git a/actions/csv/index.js b/actions/csv/index.js index 36962e38..103d0aca 100644 --- a/actions/csv/index.js +++ b/actions/csv/index.js @@ -9,8 +9,8 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -const wretch = require('wretch'); const Papa = require('papaparse'); +const wretch = require('../Network.js'); const { asGenericAction } = require('../GenericAction.js'); async function main({ url }) { diff --git a/actions/target/index.js b/actions/target/index.js index 24552504..ac7a696d 100644 --- a/actions/target/index.js +++ b/actions/target/index.js @@ -9,7 +9,7 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -const wretch = require('wretch'); +const wretch = require('../Network.js'); const { asGenericAction } = require('../GenericAction.js'); const MIN_DESCRIPTION_LENGTH = 5; @@ -18,7 +18,7 @@ async function main({ __ow_headers: headers, org, TARGET_API_KEY }) { const { authorization } = headers; const accessToken = authorization.split(' ')[1]; - const json = await wretch(`https://mc.adobe.io/${org}/target/audiences`) + const response = await wretch(`https://mc.adobe.io/${org}/target/audiences`) .headers({ 'x-api-key': TARGET_API_KEY, }) @@ -27,11 +27,11 @@ async function main({ __ow_headers: headers, org, TARGET_API_KEY }) { .get() .json(); - if (!json.audiences) { + if (!response.audiences) { throw new Error('Failed to fetch audiences'); } - return json.audiences + return response.audiences .filter((audience) => audience.name && audience.type === 'reusable') .map((audience) => ({ id: audience.id, diff --git a/app.config.yaml b/app.config.yaml index f7162f6e..1de0b2e1 100644 --- a/app.config.yaml +++ b/app.config.yaml @@ -19,7 +19,7 @@ application: IMS_SERVICE_PERM_AUTH_CODE: $IMS_SERVICE_PERM_AUTH_CODE IMS_PRODUCT_CONTEXT: $IMS_PRODUCT_CONTEXT limits: - timeout: 180000 + timeout: 60000 feedback: function: actions/feedback/index.js web: true @@ -34,7 +34,7 @@ application: IMS_SERVICE_PERM_AUTH_CODE: $IMS_SERVICE_PERM_AUTH_CODE IMS_PRODUCT_CONTEXT: $IMS_PRODUCT_CONTEXT limits: - timeout: 180000 + timeout: 60000 target: function: actions/target/index.js web: true diff --git a/e2e/WebApp.test.js b/e2e/WebApp.test.js index 028e34f8..ce05bff6 100644 --- a/e2e/WebApp.test.js +++ b/e2e/WebApp.test.js @@ -45,7 +45,7 @@ jest.mock('@adobe/exc-app/settings', () => ({ })); jest.mock('../web-src/src/helpers/NetworkHelper.js', () => ({ - wretchRetry: jest.fn().mockImplementation(() => ({ + wretch: jest.fn().mockImplementation(() => ({ get: jest.fn().mockImplementation(() => ({ json: jest.fn().mockRejectedValue(new Error('error')), })), diff --git a/web-src/src/components/GenerateButton.js b/web-src/src/components/GenerateButton.js index 3c6931b0..495fb278 100644 --- a/web-src/src/components/GenerateButton.js +++ b/web-src/src/components/GenerateButton.js @@ -39,6 +39,7 @@ export function GenerateButton() { const setIsOpenPromptEditor = useSetRecoilState(promptEditorState); const [generationInProgress, setGenerationInProgress] = useState(false); const saveResults = useSaveResults(); + const generateResults = useCallback(async () => { try { const finalPrompt = renderPrompt(prompt, parameters); diff --git a/web-src/src/helpers/NetworkHelper.js b/web-src/src/helpers/NetworkHelper.js index 8c6d03dc..5fe91573 100644 --- a/web-src/src/helpers/NetworkHelper.js +++ b/web-src/src/helpers/NetworkHelper.js @@ -10,12 +10,28 @@ * governing permissions and limitations under the License. */ import wretch from 'wretch'; -import { retry } from 'wretch/middlewares/retry'; -export function wretchRetry(url) { +function unwrapError(error) { + if (error.json?.error) { + throw new Error(error.json.error); + } + console.error(`Unexpected error: ${error}`); + throw new Error('Oops! We\'ve encountered an unexpected error. Please try again later.'); +} + +function wretchWithOptions(url) { return wretch(url) - .middlewares([retry({ - retryOnNetworkError: true, - until: (response) => response && (response.ok || (response.status >= 400 && response.status < 500)), - })]); + .headers({ 'X-OW-EXTRA-LOGGING': 'on' }) + .resolve((resolver) => { + return resolver + .badRequest((err) => unwrapError(err)) + .unauthorized((err) => unwrapError(err)) + .forbidden((err) => unwrapError(err)) + .notFound((err) => unwrapError(err)) + .timeout((err) => unwrapError(err)) + .internalError((err) => unwrapError(err)) + .fetchError((err) => unwrapError(err)); + }); } + +export { wretchWithOptions as wretch }; diff --git a/web-src/src/services/CsvParserService.js b/web-src/src/services/CsvParserService.js index 929f53a9..ea0cbc8c 100644 --- a/web-src/src/services/CsvParserService.js +++ b/web-src/src/services/CsvParserService.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ import QueryStringAddon from 'wretch/addons/queryString'; -import { wretchRetry } from '../../../actions/Network.js'; +import { wretch } from '../helpers/NetworkHelper.js'; export class CsvParserService { constructor({ @@ -22,7 +22,7 @@ export class CsvParserService { } async getData(url) { - const json = await wretchRetry(this.csvParserEndpoint) + const json = await wretch(this.csvParserEndpoint) .addon(QueryStringAddon) .query({ url }) .get() diff --git a/web-src/src/services/FirefallService.js b/web-src/src/services/FirefallService.js index bed95a9a..d83166d2 100644 --- a/web-src/src/services/FirefallService.js +++ b/web-src/src/services/FirefallService.js @@ -9,7 +9,7 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -import { wretchRetry } from '../helpers/NetworkHelper.js'; +import { wretch } from '../helpers/NetworkHelper.js'; export class FirefallService { constructor({ @@ -29,7 +29,7 @@ export class FirefallService { async complete(prompt, temperature) { /* eslint-disable-next-line camelcase */ - const { query_id, generations } = await wretchRetry(this.completeEndpoint) + const { query_id, generations } = await wretch(this.completeEndpoint) .post({ prompt, temperature, @@ -46,14 +46,13 @@ export class FirefallService { async feedback(queryId, sentiment) { /* eslint-disable-next-line camelcase */ - const { feedback_id } = await wretchRetry(this.feedbackEndpoint) + const { feedback_id } = await wretch(this.feedbackEndpoint) .post({ queryId, sentiment, imsOrg: this.imsOrg, accessToken: this.accessToken, - }) - .json(); + }); /* eslint-disable-next-line camelcase */ return feedback_id; } diff --git a/web-src/src/services/TargetService.js b/web-src/src/services/TargetService.js index 68eb4b68..d9903f27 100644 --- a/web-src/src/services/TargetService.js +++ b/web-src/src/services/TargetService.js @@ -9,7 +9,7 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -import { wretchRetry } from '../../../actions/Network.js'; +import { wretch } from '../helpers/NetworkHelper.js'; export class TargetService { constructor({ @@ -26,7 +26,7 @@ export class TargetService { async getAudiences() { const url = `${this.targetEndpoint}?org=${this.imsTenant}`; - return wretchRetry(url) + return wretch(url) .auth(`Bearer ${this.accessToken}`) .accept('application/json') .get()