#3889 - Increase Logging for SFTP and file handling #3897
Conversation
There was a problem hiding this comment.
Good Job @andrewsignori-aot . 2 minor comments and a question.
| } catch (error) { | ||
| this.logger.error(`Error uploading file ${remoteFilePath}.`, error); | ||
| throw error; |
sources/packages/backend/libs/integrations/src/services/ssh/sftp-integration-base.ts
Outdated
Show resolved
Hide resolved
...ckages/backend/libs/integrations/src/sfas-integration/sfas-integration.processing.service.ts
Show resolved
Hide resolved
| error(message: unknown, error?: unknown, context?: string): void { | ||
| const errorBuilder = new StringBuilder(); | ||
| if (typeof message === "string") { | ||
| errorBuilder.appendLine(message); | ||
| } else { | ||
| errorBuilder.appendLine(parseJSONError(message)); | ||
| } | ||
| if (error) { | ||
| errorBuilder.appendLine(parseJSONError(error)); | ||
| } | ||
| super.error(errorBuilder.toString(), context); | ||
| } | ||
|
|
There was a problem hiding this comment.
Asking for my understanding, how badly it looks in comparison when we use the super class methods OOTB?
There was a problem hiding this comment.
For my understanding as well.
There was a problem hiding this comment.
Please see below the details.
Without the change using the below.
this.logger.error(`Error uploading file ${remoteFilePath}.`);
this.logger.error(error);Results
Using the below.
this.logger.error(`Error uploading file ${remoteFilePath}.`, error);Results
Using the PR code.
this.logger.error(`Error uploading file ${remoteFilePath}.`, error);
this.logger.error(`Error uploading file ${remoteFilePath}.`, error, "SOME CONTEXT");
There was a problem hiding this comment.
To be honest the parseJSONError was a quick implementation from the past to try to print errors and causes recursively.
It can be done in a better way but it is the method that we have right now.
sources/packages/backend/libs/utilities/src/logger/logger.service.ts
Outdated
Show resolved
Hide resolved
| try { | ||
| this.logger.log(`Uploading ${remoteFilePath}`); | ||
| client = await this.getClient(); |
|
Backend Unit Tests Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E SIMS API Coverage Report
|
There was a problem hiding this comment.
Nice work @andrewsignori-aot 👍 Thanks for explaining the PR comment.
Main PR Goal
SFTPIntegrationBasewithout disrupting the consumer's methods and try to limit the amount of refactoring.Changes
LoggerService.errorto receive a friendly error and log the error itself, keeping it compatible with the existing methods.SFTPIntegrationBase. The errors are reshown keeping the current behavior for current method consumers.mais.tsthat was using the log context.SFTPIntegrationBasemethod is now generating more logs, the archive operation error handling was adjusted to ensure the exception details are logged.SFTP_ARCHIVE_DIRECTORYfor every archive method.