Skip to content
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.

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

chore: Improve error handling by using ErrorWithCause #581

Merged
merged 32 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
9cc6014
feat: use ErrorWithCause for getAllDeployments
ZhongpinWang Mar 4, 2025
21b4697
feat: improve error handling in orchestration
ZhongpinWang Mar 4, 2025
6201330
chore: clean up error message
ZhongpinWang Mar 4, 2025
d489426
refactor: throw ErrorWithCause at the root executeRequest
ZhongpinWang Mar 4, 2025
3308d2a
refactor: improve error handling
ZhongpinWang Mar 4, 2025
2cab53d
fix: replace system with user
ZhongpinWang Mar 4, 2025
c22669b
chore: improve sample code
ZhongpinWang Mar 4, 2025
1d4f0d4
fix: lock
ZhongpinWang Mar 4, 2025
26634ec
fix: lint
ZhongpinWang Mar 4, 2025
a492d7c
chore: remove unnecessary error with cause test
ZhongpinWang Mar 4, 2025
f3675c2
fix: import
ZhongpinWang Mar 4, 2025
5d98755
refactor
ZhongpinWang Mar 4, 2025
ad98648
chore: add doc for using ErrorWithCause
ZhongpinWang Mar 4, 2025
14a475c
improve doc
ZhongpinWang Mar 4, 2025
7dfb13e
Merge branch 'main' into improve-error-handling-by-using-error-with-c…
ZhongpinWang Mar 4, 2025
97ef240
fix: lint
ZhongpinWang Mar 4, 2025
b569fed
chore: add changeset
ZhongpinWang Mar 4, 2025
8e991c7
changeset
ZhongpinWang Mar 4, 2025
a57567b
fix: test
ZhongpinWang Mar 4, 2025
24ea6cb
fix: Changes from lint
Mar 4, 2025
87a05c4
changeset
ZhongpinWang Mar 4, 2025
07754a9
fix: changeset
ZhongpinWang Mar 4, 2025
3bed91e
reorder local testing and error handling
ZhongpinWang Mar 5, 2025
c20b4e8
reorder
ZhongpinWang Mar 5, 2025
763d38a
improve error handling doc
ZhongpinWang Mar 5, 2025
01ed7b1
chore: change changeset to Fixed Issue
ZhongpinWang Mar 5, 2025
8fbb1c3
doc: improve orchestration content filtering error handling
ZhongpinWang Mar 5, 2025
c901e8c
changeset
ZhongpinWang Mar 5, 2025
d80c7f1
Merge branch 'main' into improve-error-handling-by-using-error-with-c…
ZhongpinWang Mar 5, 2025
022cb05
Merge branch 'main' into improve-error-handling-by-using-error-with-c…
ZhongpinWang Mar 6, 2025
5b623e7
Merge branch 'main' into improve-error-handling-by-using-error-with-c…
ZhongpinWang Mar 6, 2025
845f448
doc: improve error handling doc
ZhongpinWang Mar 6, 2025
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
3 changes: 2 additions & 1 deletion packages/ai-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
},
"dependencies": {
"@sap-ai-sdk/core": "workspace:^",
"@sap-cloud-sdk/connectivity": "^3.26.1"
"@sap-cloud-sdk/connectivity": "^3.26.1",
"@sap-cloud-sdk/util": "^3.26.1"
}
}
45 changes: 43 additions & 2 deletions packages/ai-api/src/utils/deployment-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import {
aiCoreDestination,
mockDeploymentsList
} from '../../../../test-util/mock-http.js';
import { type AiDeployment } from '../client/AI_CORE_API';
import { resolveDeploymentId } from './deployment-resolver.js';
import { type AiDeployment } from '../client/AI_CORE_API/index.js';
import {
getAllDeployments,
resolveDeploymentId
} from './deployment-resolver.js';
import { deploymentCache } from './deployment-cache.js';

describe('deployment resolver', () => {
Expand Down Expand Up @@ -114,6 +117,44 @@ describe('deployment resolver', () => {

expect(id).toEqual('5');
});

describe('get all deployments', () => {
it('should return all deployments', async () => {
mockResponse();
const expected = [
{
id: '1',
details: {
resources: {
backendDetails: {
model: {
name: 'gpt-4o',
version: 'latest'
}
}
}
}
},
{
id: '2',
details: {
resources: {
backendDetails: {
model: {
name: 'gpt-4o',
version: '0613'
}
}
}
}
}
];
const deployments = await getAllDeployments({
scenarioId: 'foundation-models'
});
expect(deployments).toStrictEqual(expected);
});
});
});

function mockResponse() {
Expand Down
13 changes: 10 additions & 3 deletions packages/ai-api/src/utils/deployment-resolver.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ErrorWithCause } from '@sap-cloud-sdk/util';
import {
type AiDeployment,
DeploymentApi
Expand Down Expand Up @@ -138,7 +139,13 @@ export async function resolveDeploymentId(
return deployments[0].id;
}

async function getAllDeployments(
/**
* Get all deployments that match the given criteria.
* @param opts - The options for the deployment resolution.
* @returns A promise of an array of deployments.
* @internal
*/
export async function getAllDeployments(
opts: DeploymentResolutionOptions
): Promise<AiDeployment[]> {
const {
Expand All @@ -160,8 +167,8 @@ async function getAllDeployments(
deploymentCache.setAll(opts, resources);

return resources;
} catch (error) {
throw new Error('Failed to fetch the list of deployments: ' + error);
} catch (error: any) {
throw new ErrorWithCause('Failed to fetch the list of deployments.', error);
}
}

Expand Down
23 changes: 16 additions & 7 deletions packages/core/src/http-client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
ErrorWithCause,
mergeIgnoreCase,
removeLeadingSlashes,
removeTrailingSlashes
Expand Down Expand Up @@ -66,13 +67,21 @@ export async function executeRequest(
data: JSON.stringify(data)
};

return executeHttpRequest(
{ ...aiCoreDestination, url: getTargetUrl(aiCoreDestination.url, url) },
mergedRequestConfig,
{
fetchCsrfToken: false
}
);
try {
const response = await executeHttpRequest(
{ ...aiCoreDestination, url: getTargetUrl(aiCoreDestination.url, url) },
mergedRequestConfig,
{
fetchCsrfToken: false
}
);
return response;
} catch (error: any) {
throw new ErrorWithCause(
`Request failed with status code ${error.status}.`,
error
);
}
}

function mergeWithDefaultRequestConfig(
Expand Down
29 changes: 22 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 27 additions & 3 deletions sample-code/README.md
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some docs were missing.

Also mask grounding input should have been placed as part of the orchestration group.

Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ Get chat completion response for a given static input.
Get chat completion response with template and input parameters.
Define variable by wrapping it with `{{?...}}`.

#### Image Recognition
#### Templating with Reference

`GET /orchestration/image`
`GET /orchestration/templateRef`

Get chat completion response with image input.
Get chat completion response with template reference and input parameters.

#### Input Filtering

Expand All @@ -162,6 +162,30 @@ For example, use `buildAzureContentSafetyFilter()` function to build Azure conte

Send chat completion request with a custom header as the custom request configuration.

#### Orchestration Client from JSON config

`GET /orchestration/fromJson`

Get chat completion response with a JSON config initialized orchestration client.

#### Image Recognition

`GET /orchestration/image`

Get chat completion response with image input.

#### Structured Response

`GET /orchestration/responseFormat`

Get chat completion response with structured response format.

#### Mask Grounding Input

`GET /orchestration/maskGroundingInput`

Get chat completion response with masked grounding input.

#### Chat Completion Streaming

`POST /orchestration-stream/chat-completion-stream`
Expand Down
9 changes: 6 additions & 3 deletions sample-code/src/orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import type {
OrchestrationStreamChunkResponse,
OrchestrationStreamResponse,
OrchestrationResponse,
StreamOptions
StreamOptions,
ErrorResponse
} from '@sap-ai-sdk/orchestration';

const logger = createLogger({
Expand Down Expand Up @@ -168,8 +169,9 @@ const templating = { template: [{ role: 'user', content: '{{?input}}' }] };

/**
* Apply multiple content filters to the input.
* @returns The orchestration service error response.
*/
export async function orchestrationInputFiltering(): Promise<void> {
export async function orchestrationInputFiltering(): Promise<ErrorResponse> {
// Build Azure content filter with only safe content allowed for hate and violence
const azureContentSafetyFilter = buildAzureContentSafetyFilter({
Hate: 'ALLOW_SAFE',
Expand All @@ -196,11 +198,12 @@ export async function orchestrationInputFiltering(): Promise<void> {
});
throw new Error('Input was not filtered as expected.');
} catch (error: any) {
if (error.response?.status === 400) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to make these changes due to the introduction of ErrorWithCause

if (error.cause?.status === 400) {
logger.info('Input was filtered as expected.');
} else {
throw error;
}
return error.cause?.response?.data;
}
}

Expand Down
Loading
Loading