Skip to content

Conversation

@zongqichen
Copy link
Contributor

Summary

Fixes #308

This PR ensures that errors during cds build properly terminate the build process by throwing a BuildError instead of silently logging errors.

Changes

Core Fix (lib/build.js)

  • Add BuildError import from cds.build
  • Wrap build() method in try-catch and throw BuildError on failure
  • Remove unnecessary inner try-catch blocks in _writeResourcesFiles - let errors bubble up naturally
  • Refactor code for better readability (_createProgressBar(), _countTotalFiles() helper methods)

Tests

  • Unit tests (__tests__/unit/build.test.js): Updated to expect thrown errors instead of silent handling
  • Integration tests (__tests__/integration/cds-build.test.js): Added 7 new test cases
    • Successful build: exit code 0, ord-document.json generated, correct file structure, file size > 0
    • Failed build: exit code non-0, error message output

Configuration

  • integration-test-app/package.json: Added @cap-js/ord dependency to enable build plugin discovery
  • package.json: Added test:integration:build npm script

Test Results

  • ✅ 367 unit tests passed
  • ✅ 7 integration tests passed

Design Philosophy

Follows CAP Build Plugin design philosophy:

  • this.pushMessage(message, severity) - for non-fatal info/warnings
  • throw new BuildError(message) - for critical errors that should terminate the build

Fixes #308

Changes:
- Add BuildError import from cds.build in lib/build.js
- Wrap build() method in try-catch and throw BuildError on failure
- Remove unnecessary inner try-catch blocks in _writeResourcesFiles
- Refactor code for better readability (_createProgressBar, _countTotalFiles)
- Update unit tests to expect thrown errors instead of silent handling
- Add cds build integration tests (cds-build.test.js)
- Add @cap-js/ord dependency to integration-test-app for build plugin discovery
- Add test:integration:build npm script
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot left a comment

Choose a reason for hiding this comment

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

The pull request improves error handling by ensuring build failures terminate the process properly. However, there are three substantive issues: (1) Promise.all may process partial results on early errors, (2) potential TypeError when resourceDefinitions is undefined, and (3) a test that doesn't reflect the actual wrapped error behavior.

PR Bot Information

Version: 1.17.30 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • LLM: anthropic--claude-4.5-sonnet
  • Correlation ID: 7a7e3950-f7b1-11f0-84c5-e4cc77060242
  • Event Trigger: issue_comment.created

@hyperspace-insights hyperspace-insights bot deleted a comment from zongqichen Jan 22, 2026
@zongqichen zongqichen requested a review from swennemers January 22, 2026 17:14
@zongqichen zongqichen added the run-e2e Trigger e2e test pipeline label Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-e2e Trigger e2e test pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Throw real BuildError during cds build

3 participants