Skip to content

Commit 0ba213d

Browse files
rob93crob93c
authored
Remove corrupted file answer and improve logs (#492)
1 parent e118d68 commit 0ba213d

9 files changed

Lines changed: 57 additions & 107 deletions

File tree

‎src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package com.github.stickerifier.stickerify.bot;
22

3-
import static com.github.stickerifier.stickerify.logger.StructuredLogger.REQUEST_DETAILS;
4-
import static com.github.stickerifier.stickerify.telegram.Answer.CORRUPTED;
3+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.EXCEPTION_MESSAGE_LOG_KEY;
4+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_ID_VALUE;
5+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_PATH_LOG_KEY;
6+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.ORIGINAL_REQUEST_LOG_KEY;
7+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.REQUEST_DETAILS_VALUE;
58
import static com.github.stickerifier.stickerify.telegram.Answer.ERROR;
69
import static com.github.stickerifier.stickerify.telegram.Answer.FILE_ALREADY_VALID;
710
import static com.github.stickerifier.stickerify.telegram.Answer.FILE_READY;
@@ -10,7 +13,6 @@
1013
import static java.util.HashSet.newHashSet;
1114
import static java.util.concurrent.Executors.newThreadPerTaskExecutor;
1215

13-
import com.github.stickerifier.stickerify.exception.CorruptedFileException;
1416
import com.github.stickerifier.stickerify.exception.FileOperationException;
1517
import com.github.stickerifier.stickerify.exception.TelegramApiException;
1618
import com.github.stickerifier.stickerify.logger.StructuredLogger;
@@ -77,7 +79,7 @@ public int process(List<Update> updates) {
7779
updates.forEach(update -> executor.execute(() -> {
7880
if (update.message() != null) {
7981
var request = new TelegramRequest(update.message());
80-
ScopedValue.where(REQUEST_DETAILS, request.toRequestDetails()).run(() -> answer(request));
82+
ScopedValue.where(REQUEST_DETAILS_VALUE, request.toRequestDetails()).run(() -> answer(request));
8183
}
8284
}));
8385

@@ -86,7 +88,10 @@ public int process(List<Update> updates) {
8688

8789
@Override
8890
public void onException(TelegramException e) {
89-
LOGGER.at(Level.ERROR).setCause(e).addKeyValue("exception_message", e.getMessage()).log("An unexpected failure occurred");
91+
LOGGER.at(Level.ERROR)
92+
.setCause(e)
93+
.addKeyValue(EXCEPTION_MESSAGE_LOG_KEY, e.getMessage())
94+
.log("An unexpected failure occurred");
9095
}
9196

9297
@Override
@@ -116,7 +121,7 @@ private void answerFile(TelegramRequest request, TelegramFile file) {
116121
if (file == TelegramFile.NOT_SUPPORTED) {
117122
answerText(ERROR, request);
118123
} else if (file.canBeDownloaded()) {
119-
answerFile(request, file.id());
124+
ScopedValue.where(FILE_ID_VALUE, file.id()).run(() -> answerFile(request, file.id()));
120125
} else {
121126
LOGGER.at(Level.INFO).log("Passed-in file is too large");
122127

@@ -149,7 +154,7 @@ private void answerFile(TelegramRequest request, String fileId) {
149154
} catch (InterruptedException e) {
150155
Thread.currentThread().interrupt();
151156
} catch (Exception e) {
152-
processFailure(request, e, fileId);
157+
processFailure(request, e);
153158
} finally {
154159
deleteTempFiles(pathsToDelete);
155160
}
@@ -169,21 +174,16 @@ private File retrieveFile(String fileId) throws TelegramApiException, FileOperat
169174
}
170175
}
171176

172-
private void processFailure(TelegramRequest request, Exception e, String fileId) {
177+
private void processFailure(TelegramRequest request, Exception e) {
173178
if (e instanceof TelegramApiException telegramException) {
174179
boolean replyToUser = processTelegramFailure(telegramException, false);
175180
if (!replyToUser) {
176181
return;
177182
}
178183
}
179184

180-
if (e instanceof CorruptedFileException) {
181-
LOGGER.at(Level.INFO).log("Unable to reply to the request: the file is corrupted");
182-
answerText(CORRUPTED, request);
183-
} else {
184-
LOGGER.at(Level.WARN).setCause(e).addKeyValue("file_id", fileId).log("Unable to process file");
185-
answerText(ERROR, request);
186-
}
185+
LOGGER.at(Level.ERROR).setCause(e).log("Unable to process file");
186+
answerText(ERROR, request);
187187
}
188188

189189
private boolean processTelegramFailure(TelegramApiException e, boolean logUnmatchedFailure) {
@@ -206,7 +206,7 @@ private boolean processTelegramFailure(TelegramApiException e, boolean logUnmatc
206206
private void answerText(TelegramRequest request) {
207207
var message = request.message();
208208
if (message.text() == null) {
209-
LOGGER.at(Level.INFO).log("An unhandled message type has been received");
209+
LOGGER.at(Level.INFO).addKeyValue(ORIGINAL_REQUEST_LOG_KEY, message).log("An unhandled message type has been received");
210210
}
211211

212212
answerText(request.getAnswerMessage(), request);
@@ -241,10 +241,10 @@ private static void deleteTempFiles(Set<Path> pathsToDelete) {
241241
for (var path : pathsToDelete) {
242242
try {
243243
if (!Files.deleteIfExists(path)) {
244-
LOGGER.at(Level.INFO).addKeyValue("file_path", path).log("Unable to delete temp file");
244+
LOGGER.at(Level.INFO).addKeyValue(FILE_PATH_LOG_KEY, path).log("Unable to delete temp file");
245245
}
246246
} catch (IOException e) {
247-
LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_path", path).log("An error occurred trying to delete temp file");
247+
LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, path).log("An error occurred trying to delete temp file");
248248
}
249249
}
250250
}

‎src/main/java/com/github/stickerifier/stickerify/exception/CorruptedFileException.java‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎src/main/java/com/github/stickerifier/stickerify/logger/StructuredLogger.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,40 @@
88

99
public record StructuredLogger(Logger logger) {
1010

11-
public static final ScopedValue<RequestDetails> REQUEST_DETAILS = ScopedValue.newInstance();
12-
public static final ScopedValue<String> MIME_TYPE = ScopedValue.newInstance();
11+
public static final ScopedValue<RequestDetails> REQUEST_DETAILS_VALUE = ScopedValue.newInstance();
12+
public static final ScopedValue<String> FILE_ID_VALUE = ScopedValue.newInstance();
13+
public static final ScopedValue<String> MIME_TYPE_VALUE = ScopedValue.newInstance();
14+
15+
private static final String REQUEST_DETAILS_LOG_KEY = "request_details";
16+
private static final String FILE_ID_LOG_KEY = "file_id";
17+
private static final String MIME_TYPE_LOG_KEY = "mime_type";
18+
public static final String EXCEPTION_MESSAGE_LOG_KEY = "exception_message";
19+
public static final String ORIGINAL_REQUEST_LOG_KEY = "original_request";
20+
public static final String FILE_PATH_LOG_KEY = "file_path";
21+
public static final String STICKER_LOG_KEY = "sticker";
1322

1423
public StructuredLogger(Class<?> clazz) {
1524
this(LoggerFactory.getLogger(clazz));
1625
}
1726

1827
/**
19-
* Creates a {@link LoggingEventBuilder} at the specified level with request details and MIME type information, if set.
28+
* Creates a {@link LoggingEventBuilder} at the specified level enriched with request details,
29+
* file id, and MIME type information when available.
2030
*
2131
* @param level the level of the log
2232
* @return the log builder with context information
2333
*/
2434
public LoggingEventBuilder at(Level level) {
2535
var logBuilder = logger.atLevel(level);
2636

27-
if (REQUEST_DETAILS.isBound()) {
28-
logBuilder = logBuilder.addKeyValue("request_details", REQUEST_DETAILS.get());
37+
if (REQUEST_DETAILS_VALUE.isBound()) {
38+
logBuilder = logBuilder.addKeyValue(REQUEST_DETAILS_LOG_KEY, REQUEST_DETAILS_VALUE.get());
39+
}
40+
if (FILE_ID_VALUE.isBound()) {
41+
logBuilder = logBuilder.addKeyValue(FILE_ID_LOG_KEY, FILE_ID_VALUE.get());
2942
}
30-
if (MIME_TYPE.isBound()) {
31-
logBuilder = logBuilder.addKeyValue("mime_type", MIME_TYPE.get());
43+
if (MIME_TYPE_VALUE.isBound()) {
44+
logBuilder = logBuilder.addKeyValue(MIME_TYPE_LOG_KEY, MIME_TYPE_VALUE.get());
3245
}
3346

3447
return logBuilder;

‎src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.github.stickerifier.stickerify.media;
22

3-
import static com.github.stickerifier.stickerify.logger.StructuredLogger.MIME_TYPE;
3+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_PATH_LOG_KEY;
4+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.MIME_TYPE_VALUE;
5+
import static com.github.stickerifier.stickerify.logger.StructuredLogger.STICKER_LOG_KEY;
46
import static com.github.stickerifier.stickerify.media.MediaConstraints.MATROSKA_FORMAT;
57
import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_ANIMATION_DURATION_SECONDS;
68
import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_ANIMATION_FILE_SIZE;
@@ -14,7 +16,6 @@
1416
import static java.nio.charset.StandardCharsets.ISO_8859_1;
1517
import static java.nio.charset.StandardCharsets.UTF_8;
1618

17-
import com.github.stickerifier.stickerify.exception.CorruptedFileException;
1819
import com.github.stickerifier.stickerify.exception.FileOperationException;
1920
import com.github.stickerifier.stickerify.exception.MediaException;
2021
import com.github.stickerifier.stickerify.exception.ProcessException;
@@ -67,7 +68,7 @@ public final class MediaHelper {
6768
public static @Nullable File convert(File inputFile) throws Exception {
6869
var mimeType = detectMimeType(inputFile);
6970

70-
return ScopedValue.where(MIME_TYPE, mimeType).call(() -> performConversion(inputFile, mimeType));
71+
return ScopedValue.where(MIME_TYPE_VALUE, mimeType).call(() -> performConversion(inputFile, mimeType));
7172
}
7273

7374
/**
@@ -81,7 +82,7 @@ private static String detectMimeType(File file) throws MediaException {
8182
try {
8283
return TIKA.detect(file);
8384
} catch (IOException e) {
84-
LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_name", file.getName()).log("Unable to retrieve MIME type");
85+
LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, file.getPath()).log("Unable to retrieve MIME type");
8586
throw new MediaException(e);
8687
}
8788
}
@@ -144,10 +145,10 @@ private static boolean isSupportedVideo(String mimeType) {
144145
*
145146
* @param file the file to check
146147
* @return {@code true} if the file is compliant
147-
* @throws FileOperationException if an error occurred retrieving the size of the file
148+
* @throws MediaException if an error occurred retrieving video information
148149
* @throws InterruptedException if the current thread is interrupted while retrieving file info
149150
*/
150-
private static boolean isVideoCompliant(File file) throws FileOperationException, CorruptedFileException, InterruptedException {
151+
private static boolean isVideoCompliant(File file) throws MediaException, InterruptedException {
151152
var mediaInfo = retrieveMultimediaInfo(file);
152153

153154
var formatInfo = mediaInfo.format();
@@ -178,10 +179,10 @@ private static boolean isVideoCompliant(File file) throws FileOperationException
178179
*
179180
* @param file the video to check
180181
* @return passed-in video's multimedia information
181-
* @throws CorruptedFileException if an error occurred retrieving file information
182+
* @throws MediaException if an error occurred retrieving file information
182183
* @throws InterruptedException if the current thread is interrupted while retrieving file info
183184
*/
184-
static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileException, InterruptedException {
185+
static MultimediaInfo retrieveMultimediaInfo(File file) throws MediaException, InterruptedException {
185186
var command = new String[] {
186187
"ffprobe",
187188
"-hide_banner",
@@ -197,7 +198,7 @@ static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileExce
197198

198199
return GSON.fromJson(output, MultimediaInfo.class);
199200
} catch (ProcessException | JsonSyntaxException e) {
200-
throw new CorruptedFileException("The file could not be processed successfully", e);
201+
throw new MediaException("Unable to retrieve media information", e);
201202
}
202203
}
203204

@@ -253,7 +254,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th
253254
try (var gzipInputStream = new GZIPInputStream(new FileInputStream(file))) {
254255
uncompressedContent = new String(gzipInputStream.readAllBytes(), UTF_8);
255256
} catch (IOException e) {
256-
LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_name", file.getName()).log("Unable to retrieve gzip content");
257+
LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, file.getPath()).log("Unable to retrieve gzip content");
257258
}
258259

259260
try {
@@ -267,7 +268,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th
267268
}
268269
}
269270

270-
LOGGER.at(Level.WARN).addKeyValue("sticker", sticker).log("The animated sticker doesn't meet Telegram's requirements");
271+
LOGGER.at(Level.WARN).addKeyValue(STICKER_LOG_KEY, sticker).log("The animated sticker doesn't meet Telegram's requirements");
271272
} catch (JsonSyntaxException _) {
272273
LOGGER.at(Level.INFO).log("The archive isn't an animated sticker");
273274
}
@@ -354,10 +355,10 @@ private static boolean isAnimatedWebp(File file) {
354355
* @param image the image to check
355356
* @param mimeType the MIME type of the file
356357
* @return {@code true} if the file is compliant
357-
* @throws CorruptedFileException if an error occurred retrieving image information
358+
* @throws MediaException if an error occurred retrieving image information
358359
* @throws InterruptedException if the current thread is interrupted while retrieving file info
359360
*/
360-
private static boolean isImageCompliant(File image, String mimeType) throws CorruptedFileException, InterruptedException {
361+
private static boolean isImageCompliant(File image, String mimeType) throws MediaException, InterruptedException {
361362
var mediaInfo = retrieveMultimediaInfo(image);
362363

363364
var formatInfo = mediaInfo.format();
@@ -448,7 +449,7 @@ private static File createTempFile(String fileExtension) throws FileOperationExc
448449
private static void deleteFile(File file) throws FileOperationException {
449450
try {
450451
if (!Files.deleteIfExists(file.toPath())) {
451-
LOGGER.at(Level.INFO).addKeyValue("file_path", file.toPath()).log("Unable to delete file");
452+
LOGGER.at(Level.INFO).addKeyValue(FILE_PATH_LOG_KEY, file.toPath()).log("Unable to delete file");
452453
}
453454
} catch (IOException e) {
454455
throw new FileOperationException("An error occurred deleting the file", e);
@@ -493,11 +494,10 @@ private static File convertToWebm(File file) throws MediaException, InterruptedE
493494
}
494495
throw new MediaException("FFmpeg two-pass conversion failed", e);
495496
} finally {
496-
var logFileName = logPrefix + "-0.log";
497497
try {
498-
deleteFile(new File(logFileName));
498+
deleteFile(new File(logPrefix + "-0.log"));
499499
} catch (FileOperationException e) {
500-
LOGGER.at(Level.WARN).setCause(e).addKeyValue("file_name", logFileName).log("Could not delete log file");
500+
LOGGER.at(Level.WARN).setCause(e).log("Could not delete log file");
501501
}
502502
}
503503

‎src/main/java/com/github/stickerifier/stickerify/telegram/Answer.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,6 @@ public enum Answer {
3535
ERROR("""
3636
The file conversion was unsuccessful: only images, gifs, standard and video stickers are supported\\.
3737
38-
If you think it should have worked, please report the issue on [Github](https://github.com/Stickerifier/Stickerify/issues/new/choose)\\.
39-
""", true),
40-
CORRUPTED("""
41-
The conversion was unsuccessful: the video might be corrupted and it cannot be processed\\.
42-
4338
If you think it should have worked, please report the issue on [Github](https://github.com/Stickerifier/Stickerify/issues/new/choose)\\.
4439
""", true),
4540
PRIVACY_POLICY("""

‎src/main/resources/logback.xml‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
<configuration>
22
<statusListener class="ch.qos.logback.core.status.NopStatusListener"/>
33

4-
<appender name="JSON_CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
4+
<appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
55
<encoder class="net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder">
66
<providers>
7-
<timestamp>
8-
<timeZone>UTC</timeZone>
9-
</timestamp>
107
<logLevel/>
118
<threadName/>
129
<pattern>
@@ -24,7 +21,7 @@
2421
</appender>
2522

2623
<root level="${LOG_LEVEL:-INFO}">
27-
<appender-ref ref="JSON_CONSOLE"/>
24+
<appender-ref ref="CONSOLE"/>
2825
</root>
2926

3027
<logger name="org.apache.tika" level="INFO"/>

‎src/test/java/com/github/stickerifier/stickerify/bot/MockResponses.java‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -295,30 +295,6 @@ public final class MockResponses {
295295
}
296296
""").build();
297297

298-
static final MockResponse CORRUPTED_FILE = new MockResponse.Builder().body("""
299-
{
300-
ok: true,
301-
result: [
302-
{
303-
update_id: 1,
304-
message: {
305-
message_id: 1,
306-
from: {
307-
id: 123456
308-
},
309-
chat: {
310-
id: 1
311-
},
312-
video: {
313-
file_id: "corrupted.mp4",
314-
file_size: 200000
315-
}
316-
}
317-
}
318-
]
319-
}
320-
""").build();
321-
322298
static MockResponse fileInfo(String fileName) {
323299
return new MockResponse.Builder().body("""
324300
{

‎src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -290,28 +290,4 @@ void documentNotSupported() throws Exception {
290290
assertResponseContainsMessage(sendMessage, Answer.ERROR);
291291
}
292292
}
293-
294-
@Test
295-
void corruptedVideo() throws Exception {
296-
server.enqueue(MockResponses.CORRUPTED_FILE);
297-
server.enqueue(MockResponses.fileInfo("corrupted.mp4"));
298-
server.enqueue(MockResponses.fileDownload("corrupted.mp4"));
299-
300-
try (var _ = runBot()) {
301-
var getUpdates = server.takeRequest();
302-
assertEquals("/api/token/getUpdates", getUpdates.getTarget());
303-
304-
var getFile = server.takeRequest();
305-
assertEquals("/api/token/getFile", getFile.getTarget());
306-
assertNotNull(getFile.getBody());
307-
assertEquals("file_id=corrupted.mp4", getFile.getBody().utf8());
308-
309-
var download = server.takeRequest();
310-
assertEquals("/files/token/corrupted.mp4", download.getTarget());
311-
312-
var sendMessage = server.takeRequest();
313-
assertEquals("/api/token/sendMessage", sendMessage.getTarget());
314-
assertResponseContainsMessage(sendMessage, Answer.CORRUPTED);
315-
}
316-
}
317293
}

‎src/test/resources/corrupted.mp4‎

-2.52 MB
Binary file not shown.

0 commit comments

Comments
 (0)