context: Two javascript classes in separate files, each integrating a different external service and being called in a express.js router.

See "problematic code" below:

• route

routes.post('/aws', upload.single('file'), async (req, res) => {
    const transcribeParams = JSON.parse(req.body.options)
    const bucket = 'bucket-name'

    const data = await ( await ( await awsTranscribe.Upload(req.file, bucket)).CreateJob(transcribeParams)).GetJob()

    res.send(data) 
})  

• S3 class

class AmazonS3 {
    constructor() {
        this.Upload = this.Upload
    }

    async Upload(file, bucket) {
        const uploadParams = {
            Bucket: bucket,
            Body: fs.createReadStream(file.path),
            Key: file.filename,
        }

        this.data = await s3.upload(uploadParams).promise()

        return this
    }
}  

• Transcribe class

class Transcribe extends AwsS3 {
    constructor() {
        super()
        this.CreateJob = this.CreateJob
        this.GetJob = this.GetJob
    }

    async CreateJob(params) {
        if(this.data?.Location) {
            params.Media = { ...params.Media, MediaFileUri: this.data.Location }
        }

        this.data = await transcribeService.startTranscriptionJob(params).promise()

        return this
    }

    async GetJob(jobName) {
        if(this.data?.TranscriptionJob?.TranscriptionJobName) {
            jobName = this.data.TranscriptionJob.TranscriptionJobName
        }

        this.data = await transcribeService.getTranscriptionJob({TranscriptionJobName: jobName}).promise()

        return this
    }
}

problem: the problem is with the chained awaits in the router file:
await ( await ( await awsTranscribe.Upload...
Yes, it does work, but it would be horrible for another person to maintain this code in the future.
How can i make so it would be just
awsTranscribe.Upload(req.file, bucket).CreateJob(transcribeParams).GetJob() without the .then?

CodePudding user response：

The problem is with the chained awaits in the router file: await ( await ( await awsTranscribe.Upload...

No, that's fine. In particular it would be trivial to refactor it to separate lines:

routes.post('/aws', upload.single('file'), async (req, res) => {
    const transcribeParams = JSON.parse(req.body.options)
    const bucket = 'bucket-name'

    const a = await awsTranscribe.Upload(req.file, bucket);
    const b = await b.CreateJob(transcribeParams);
    const c = await b.GetJob();

    res.send(c);
});

Your actual problem is that a, b, and c all refer to the same object awsTranscribe. Your code would also "work" if it was written

routes.post('/aws', upload.single('file'), async (req, res) => {
    const transcribeParams = JSON.parse(req.body.options)
    const bucket = 'bucket-name'

    await awsTranscribe.Upload(req.file, bucket);
    await awsTranscribe.CreateJob(transcribeParams);
    await awsTranscribe.GetJob();

    res.send(awsTranscribe);
});

The horrible thing is that you are passing your data between these methods through the mutable awsTranscribe.data property - even storing different kinds of data in it at different times! One could change the order of method calls and it would completely break in non-obvious and hard-to-debug ways.

Also it seems that multiple requests share the same awsTranscribe instance. This will not work with concurrent requests. Anything is possible from just "not working" to responding with the job data from a different user (request)! You absolutely need to fix that, then look at ugly syntax later.

What you really should do is get rid of the classes. There's no reason to use stateful objects here, this is plain procedural code. Write simple functions, taking parameters and returning values:

export async function uploadFile(file, bucket) {
    const uploadParams = {
        Bucket: bucket,
        Body: fs.createReadStream(file.path),
        Key: file.filename,
    };
    const data = s3.upload(uploadParams).promise();
    return data.Location;
}

export async function createTranscriptionJob(location, params) {
    params = {
        ...params,
        Media: {
            ...params.Media,
            MediaFileUri: location,
        },
    };
    const data = await transcribeService.startTranscriptionJob(params).promise();
    return data.TranscriptionJob;
}

async function getTranscriptionJob(job) {
    const jobName = job.TranscriptionJobName;
    return transcribeService.getTranscriptionJob({TranscriptionJobName: jobName}).promise();
}

Then you can import and call them as

routes.post('/aws', upload.single('file'), async (req, res) => {
    const transcribeParams = JSON.parse(req.body.options)
    const bucket = 'bucket-name'

    const location = await uploadFile(req.file, bucket);
    const job = await createTranscriptionJob(location, transcribeParams);
    const data = await getTranscriptionJob(job);

    res.send(c);
});