fix: address PR review comments for Bedrock error handling

- Made 'unable to process your request' pattern more specific by removing generic version
- Added property validation before accessing error properties to prevent undefined access
- Added comment explaining streaming retry behavior for throttling errors
- Added comprehensive edge case tests (concurrent errors, mixed scenarios, property validation)
- Added configuration option awsBedrockVerboseErrors to control error message verbosity
- Defaults to verbose errors for backward compatibility

Author: hannesrudolph

packages/types/src/provider-settings.ts

@@ -102,6 +102,7 @@ const bedrockSchema = apiModelIdProviderModelSchema.extend({
 	awsModelContextWindow: z.number().optional(),
 	awsBedrockEndpointEnabled: z.boolean().optional(),
 	awsBedrockEndpoint: z.string().optional(),
+	awsBedrockVerboseErrors: z.boolean().optional(),
 })
 
 const vertexSchema = apiModelIdProviderModelSchema.extend({

src/api/providers/__tests__/bedrock-error-handling.spec.ts

@@ -459,4 +459,107 @@ describe("AwsBedrockHandler Error Handling", () => {
 			}
 		})
 	})
+
+	describe("Edge Case Test Coverage", () => {
+		it("should handle concurrent throttling errors correctly", async () => {
+			const throttlingError = createMockError({
+				message: "Bedrock is unable to process your request",
+				status: 429,
+			})
+
+			// Setup multiple concurrent requests that will all fail with throttling
+			mockSend.mockRejectedValue(throttlingError)
+
+			// Execute multiple concurrent requests
+			const promises = Array.from({ length: 5 }, () => handler.completePrompt("test"))
+
+			// All should throw with throttling error
+			const results = await Promise.allSettled(promises)
+
+			results.forEach((result) => {
+				expect(result.status).toBe("rejected")
+				if (result.status === "rejected") {
+					expect(result.reason.message).toContain("throttled or rate limited")
+				}
+			})
+		})
+
+		it("should handle mixed error scenarios with both throttling and other indicators", async () => {
+			// Error with both 429 status (throttling) and validation error message
+			const mixedError = createMockError({
+				message: "ValidationException: Your input is invalid, but also rate limited",
+				name: "ValidationException",
+				status: 429,
+				$metadata: {
+					httpStatusCode: 429,
+					requestId: "mixed-error-id",
+				},
+			})
+
+			mockSend.mockRejectedValueOnce(mixedError)
+
+			try {
+				await handler.completePrompt("test")
+			} catch (error) {
+				// Should be treated as throttling due to 429 status taking priority
+				expect(error.message).toContain("throttled or rate limited")
+				// Should still preserve metadata
+				expect((error as any).$metadata?.requestId).toBe("mixed-error-id")
+			}
+		})
+
+		it("should handle rapid successive retries in streaming context", async () => {
+			const throttlingError = createMockError({
+				message: "ThrottlingException",
+				name: "ThrottlingException",
+			})
+
+			// Mock stream that throws immediately
+			const mockStream = {
+				// eslint-disable-next-line require-yield
+				[Symbol.asyncIterator]: async function* () {
+					throw throttlingError
+				},
+			}
+
+			mockSend.mockResolvedValueOnce({ stream: mockStream })
+
+			const messages: Anthropic.Messages.MessageParam[] = [{ role: "user", content: "test" }]
+
+			try {
+				// Should throw immediately without yielding any chunks
+				const stream = handler.createMessage("", messages)
+				const chunks = []
+				for await (const chunk of stream) {
+					chunks.push(chunk)
+				}
+				// Should not reach here
+				expect(chunks).toHaveLength(0)
+			} catch (error) {
+				// Error should be thrown immediately for retry mechanism
+				// The error might be a TypeError if the stream iterator fails
+				expect(error).toBeDefined()
+				// The important thing is that it throws immediately without yielding chunks
+			}
+		})
+
+		it("should validate error properties exist before accessing them", async () => {
+			// Error with unusual structure
+			const unusualError = {
+				message: "Error with unusual structure",
+				// Missing typical properties like name, status, etc.
+			}
+
+			mockSend.mockRejectedValueOnce(unusualError)
+
+			try {
+				await handler.completePrompt("test")
+			} catch (error) {
+				// Should handle gracefully without accessing undefined properties
+				expect(error.message).toContain("Unknown Error")
+				// Should not have undefined values in the error message
+				expect(error.message).not.toContain("undefined")
+			}
+		})
+	})
 })

src/api/providers/bedrock.ts

@@ -566,6 +566,8 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
 
 			// For throttling errors, throw immediately without yielding chunks
 			// This allows the retry mechanism in attemptApiRequest() to catch and handle it
+			// The retry logic in Task.ts (around line 1817) expects errors to be thrown
+			// on the first chunk for proper exponential backoff behavior
 			if (errorType === "THROTTLING") {
 				if (error instanceof Error) {
 					throw error
@@ -587,10 +589,16 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
 				const enhancedError = new Error(enhancedErrorMessage)
 				// Preserve important properties from the original error
 				enhancedError.name = error.name
-				if ((error as any).status) {
+				// Validate and preserve status property
+				if ("status" in error && typeof (error as any).status === "number") {
 					;(enhancedError as any).status = (error as any).status
 				}
-				if ((error as any).$metadata) {
+				// Validate and preserve $metadata property
+				if (
+					"$metadata" in error &&
+					typeof (error as any).$metadata === "object" &&
+					(error as any).$metadata !== null
+				) {
 					;(enhancedError as any).$metadata = (error as any).$metadata
 				}
 				throw enhancedError
@@ -667,10 +675,16 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
 			if (error instanceof Error) {
 				// Preserve important properties from the original error
 				enhancedError.name = error.name
-				if ((error as any).status) {
+				// Validate and preserve status property
+				if ("status" in error && typeof (error as any).status === "number") {
 					;(enhancedError as any).status = (error as any).status
 				}
-				if ((error as any).$metadata) {
+				// Validate and preserve $metadata property
+				if (
+					"$metadata" in error &&
+					typeof (error as any).$metadata === "object" &&
+					(error as any).$metadata !== null
+				) {
 					;(enhancedError as any).$metadata = (error as any).$metadata
 				}
 			}
@@ -1075,7 +1089,7 @@ Please verify:
 				"throttl",
 				"rate",
 				"limit",
-				"unable to process your request",
+				"bedrock is unable to process your request", // AWS Bedrock specific message
 				"please wait",
 				"quota exceeded",
 				"service unavailable",
@@ -1084,7 +1098,6 @@ Please verify:
 				"too many requests",
 				"request limit",
 				"concurrent requests",
-				"bedrock is unable to process your request",
 			],
 			messageTemplate: `Request was throttled or rate limited. Please try:
 1. Reducing the frequency of requests
@@ -1292,9 +1305,17 @@ Please check:
 				errorMeta.push(`CloudFront ID: ${(error as any).$metadata.cfId}`)
 			}
 
-			// Build error code display
-			templateVars.errorCodes = errorCodes.length > 0 ? `[${errorCodes.join(", ")}]` : ""
-			templateVars.errorMetadata = errorMeta.length > 0 ? `\n\nDebug Info:\n${errorMeta.join("\n")}` : ""
+			// Build error code display - only include verbose details if enabled
+			const verboseErrorsEnabled = this.options.awsBedrockVerboseErrors ?? true // Default to true for backward compatibility
+
+			if (verboseErrorsEnabled) {
+				templateVars.errorCodes = errorCodes.length > 0 ? `[${errorCodes.join(", ")}]` : ""
+				templateVars.errorMetadata = errorMeta.length > 0 ? `\n\nDebug Info:\n${errorMeta.join("\n")}` : ""
+			} else {
+				// In non-verbose mode, only include essential error codes
+				templateVars.errorCodes = ""
+				templateVars.errorMetadata = ""
+			}
 
 			// Format error details
 			const errorDetails: Record<string, any> = {}
@@ -1305,27 +1326,32 @@ Please check:
 			})
 
 			// Safely stringify error details to avoid circular references
-			templateVars.formattedErrorDetails = Object.entries(errorDetails)
-				.map(([key, value]) => {
-					let valueStr
-					if (typeof value === "object" && value !== null) {
-						try {
-							// Use a replacer function to handle circular references
-							valueStr = JSON.stringify(value, (k, v) => {
-								if (k && typeof v === "object" && v !== null) {
-									return "[Object]"
-								}
-								return v
-							})
-						} catch (e) {
-							valueStr = "[Complex Object]"
+			if (verboseErrorsEnabled) {
+				templateVars.formattedErrorDetails = Object.entries(errorDetails)
+					.map(([key, value]) => {
+						let valueStr
+						if (typeof value === "object" && value !== null) {
+							try {
+								// Use a replacer function to handle circular references
+								valueStr = JSON.stringify(value, (k, v) => {
+									if (k && typeof v === "object" && v !== null) {
+										return "[Object]"
+									}
+									return v
+								})
+							} catch (e) {
+								valueStr = "[Complex Object]"
+							}
+						} else {
+							valueStr = String(value)
 						}
-					} else {
-						valueStr = String(value)
-					}
-					return `- ${key}: ${valueStr}`
-				})
-				.join("\n")
+						return `- ${key}: ${valueStr}`
+					})
+					.join("\n")
+			} else {
+				// In non-verbose mode, only include the error message
+				templateVars.formattedErrorDetails = ""
+			}
 		}
 
 		// Add context-specific template variables