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.

Sign up for GitHub

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

Improve error handling in transpile command #124

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improve error handling in transpile command #124

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8698ec
catch xml parsing errors in transform()
2IMT Dec 16, 2024
ebef1bc
catch other errors in transform()
2IMT Dec 16, 2024
44e5735
improve error messages in transpile()
2IMT Dec 16, 2024
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
23 changes: 17 additions & 6 deletions eo2js/src/commands/transpile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,23 @@ const needsRetranspile = function(source, transpiled) {
*/
const transform = function(tojo, options, transformations, parser) {
const text = fs.readFileSync(tojo[verified]).toString()
let xml = parser.parse(text)
let xml;
try {
xml = parser.parse(text);
} catch (e) {
throw new Error(`Failed to parse XML for ${tojo[verified]}: ${e.message}`);
}
const pth = pathFromName(xml['program']['@_name'])
const isTest = hasMeta(xml, 'tests')
const transpiled = path.resolve(options.target, dir, `${pth}.xmir`)
const dest = path.resolve(options.project, `${pth}${isTest ? '.test' : ''}.js`)
if (needsRetranspile(tojo[verified], transpiled)) {
if (!needsRetranspile(tojo[verified], transpiled)) {
if (options.verbose) {
console.log(`Skipping ${pth} - already transpiled`);
}
return;
Copy link
Member

Choose a reason for hiding this comment

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

@2IMT we're trying to avoid multiple return statements, here's why: https://www.yegor256.com/2015/08/18/multiple-return-statements-in-oop.html

}
try {
makeDirIfNotExist(transpiled.substring(0, transpiled.lastIndexOf(path.sep)))
fs.writeFileSync(transpiled, text)
xml = text
Expand All @@ -108,8 +119,8 @@ const transform = function(tojo, options, transformations, parser) {
fs.writeFileSync(dest, first['javascript'])
filtered.slice(1).forEach((obj) => fs.appendFileSync(dest, `\n${obj['javascript']}`))
}
} else if (options.verbose) {
console.log(`Skipping ${pth} - already transpiled`)
} catch (e) {
throw new Error(`Error transforming ${tojo[verified]}: ${e.message}`);
}
}

Expand All @@ -122,10 +133,10 @@ const transpile = function(options) {
const foreign = path.resolve(options['target'], options['foreign'])
console.log(`Reading foreign tojos from: ${foreign}`)
if (!fs.existsSync(foreign)) {
throw new Error(`File ${foreign} is not found`)
throw new Error(`File not found: ${foreign}. Ensure the file exists and is accessible.`)
Copy link
Member

Choose a reason for hiding this comment

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

@2IMT I don't understand the purpose of the change

}
if (!foreign.endsWith('.json')) {
throw new Error(`Only .json foreign tojos file is supported, given ${foreign.substring(foreign.lastIndexOf(path.sep))}`)
throw new Error(`Invalid foreign tojo file format: ${foreign.substring(foreign.lastIndexOf(path.sep))}. Only .json files are supported.`);
Copy link
Member

Choose a reason for hiding this comment

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

@2IMT I don't understand the purpose of the change

}
const transformations = [
'objects', 'package', 'tests', 'attrs', 'data', 'to-js'
Expand Down
Loading