Skip to content

Conversation

SkinnnyJay
Copy link

@SkinnnyJay SkinnnyJay commented Sep 13, 2025

Changes being requested

Add defensive null/undefined checks for malformed embedding responses to prevent runtime crashes when using third-party OpenAI-compatible endpoints.

Core changes:

  • Add null/undefined validation for embedding objects and data
  • Skip null embedding objects gracefully during processing
  • Throw meaningful OpenAIError with context for missing embedding data
  • Maintain full backward compatibility with existing functionality

Test coverage:

  • Add 5 new test cases covering null/undefined scenarios
  • Add helper function makeClientWithCustomResponse for custom mock responses
  • Verify error messages include proper context (item index)
  • Ensure existing functionality remains unaffected

Additional context & links

Bug: #1542

This addresses potential runtime errors when third-party providers (Ollama, LM Studio) return malformed responses with null/undefined embedding data. The current code lacks defensive checks:

// Before (vulnerable to runtime errors)
response.data.forEach((embeddingBase64Obj) => {
  const embeddingBase64Str = embeddingBase64Obj.embedding as unknown as string;
  embeddingBase64Obj.embedding = toFloat32Array(embeddingBase64Str);
});

// After (defensive with meaningful errors)
response.data.forEach((embeddingBase64Obj) => {
  if (!embeddingBase64Obj) return; // Skip null/undefined items

  const embeddingBase64Str = embeddingBase64Obj.embedding as unknown;
  if (embeddingBase64Str == null) {
    throw new OpenAIError(`Missing embedding data for item at index ${embeddingBase64Obj.index ?? 'unknown'}`);
  }
  embeddingBase64Obj.embedding = toFloat32Array(embeddingBase64Str as string);
});

Impact:

  • ✅ Prevents runtime crashes from malformed third-party responses
  • ✅ Provides meaningful error messages for debugging
  • ✅ Maintains full backward compatibility
  • ✅ No performance impact on normal operations

✅ I understand that this repository is auto-generated, and my pull request may not be merged

- Add defensive checks for null/undefined embeddingObj items
- Add null/undefined validation for embedding data with meaningful errors
- Skip null embedding objects gracefully during processing
- Throw OpenAIError with context (item index) for missing embedding data
- Maintains compatibility with existing functionality

test(embeddings): add comprehensive null/undefined test coverage

- Add tests for null embedding objects (graceful handling)
- Add tests for missing embedding data (proper error throwing)
- Add tests for mixed valid/invalid scenarios
- Add helper function makeClientWithCustomResponse for custom mock responses
- Verify error messages include proper context (item index)
- Ensure existing functionality remains unaffected
@SkinnnyJay
Copy link
Author

Ready for review..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant