Skip to content

Commit 3a4d99a

Browse files
dpol1dpol1
authored
fix: close streams and stop tracing in finally to prevent resource leaks (#1930)
- RegexURLFilterBase: use try-with-resources for classpath InputStream; add null check when resource file is not found in classpath - DebugParseFilter: add cleanup() override to close OutputStream; add SLF4J logger and replace e.printStackTrace() - HttpProtocol (Playwright): add finally block to ensure tracing is always stopped even when an exception is thrown
1 parent 9c62f2c commit 3a4d99a

3 files changed

Lines changed: 142 additions & 115 deletions

File tree

‎core/src/main/java/org/apache/stormcrawler/filtering/regex/RegexURLFilterBase.java‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.io.IOException;
2424
import java.io.InputStream;
2525
import java.io.InputStreamReader;
26-
import java.io.Reader;
2726
import java.net.URL;
2827
import java.nio.charset.StandardCharsets;
2928
import java.util.ArrayList;
@@ -80,24 +79,27 @@ private List<RegexRule> readRules(ArrayNode rulesList) {
8079
private List<RegexRule> readRules(String rulesFile) {
8180
List<RegexRule> rules = new ArrayList<>();
8281

83-
try {
84-
InputStream regexStream = getClass().getClassLoader().getResourceAsStream(rulesFile);
85-
Reader reader = new InputStreamReader(regexStream, StandardCharsets.UTF_8);
86-
BufferedReader in = new BufferedReader(reader);
87-
String line;
88-
89-
while ((line = in.readLine()) != null) {
90-
if (line.length() == 0) {
91-
continue;
92-
}
93-
RegexRule rule = createRule(line);
94-
if (rule != null) {
95-
rules.add(rule);
82+
try (InputStream regexStream = getClass().getClassLoader().getResourceAsStream(rulesFile)) {
83+
if (regexStream == null) {
84+
LOG.error("Regex filter file '{}' not found in classpath", rulesFile);
85+
return rules;
86+
}
87+
try (BufferedReader in =
88+
new BufferedReader(
89+
new InputStreamReader(regexStream, StandardCharsets.UTF_8))) {
90+
String line;
91+
while ((line = in.readLine()) != null) {
92+
if (line.length() == 0) {
93+
continue;
94+
}
95+
RegexRule rule = createRule(line);
96+
if (rule != null) {
97+
rules.add(rule);
98+
}
9699
}
97100
}
98101
} catch (IOException e) {
99-
LOG.error("There was an error reading the default-regex-filters file");
100-
e.printStackTrace();
102+
LOG.error("There was an error reading the default-regex-filters file", e);
101103
}
102104
return rules;
103105
}

‎core/src/main/java/org/apache/stormcrawler/parse/filter/DebugParseFilter.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,15 @@
2828
import org.apache.stormcrawler.parse.ParseResult;
2929
import org.apache.xml.serialize.XMLSerializer;
3030
import org.jetbrains.annotations.NotNull;
31+
import org.slf4j.Logger;
32+
import org.slf4j.LoggerFactory;
3133
import org.w3c.dom.DocumentFragment;
3234

3335
/** Dumps the DOM representation of a document into a file. */
3436
public class DebugParseFilter extends ParseFilter {
3537

38+
private static final Logger LOG = LoggerFactory.getLogger(DebugParseFilter.class);
39+
3640
private OutputStream os;
3741

3842
@Override
@@ -43,7 +47,7 @@ public void filter(String url, byte[] content, DocumentFragment doc, ParseResult
4347
serializer.serialize(doc);
4448
os.flush();
4549
} catch (IOException e) {
46-
e.printStackTrace();
50+
LOG.error("Exception while serializing DOM", e);
4751
}
4852
}
4953

@@ -53,12 +57,23 @@ public void configure(@NotNull Map<String, Object> stormConf, @NotNull JsonNode
5357
File outFile = Files.createTempFile("DOMDump", ".xml").toFile();
5458
os = FileUtils.openOutputStream(outFile);
5559
} catch (IOException e) {
56-
e.printStackTrace();
60+
LOG.error("Exception while configuring DebugParseFilter", e);
5761
}
5862
}
5963

6064
@Override
6165
public boolean needsDOM() {
6266
return true;
6367
}
68+
69+
@Override
70+
public void cleanup() {
71+
if (os != null) {
72+
try {
73+
os.close();
74+
} catch (IOException e) {
75+
LOG.error("Exception while closing output stream in DebugParseFilter", e);
76+
}
77+
}
78+
}
6479
}

‎external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java‎

Lines changed: 107 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,8 @@ public ProtocolResponse getProtocolOutput(String url, Metadata md) throws Except
184184

185185
// https://github.com/microsoft/playwright-java#is-playwright-thread-safe
186186
synchronized (this) {
187-
188-
// tracing// Start tracing before creating / navigating a page.
189-
if (md.containsKey(MD_TRACE)) {
187+
boolean isTracing = md.containsKey(MD_TRACE);
188+
if (isTracing) {
190189
context.tracing()
191190
.start(
192191
new Tracing.StartOptions()
@@ -201,109 +200,120 @@ public ProtocolResponse getProtocolOutput(String url, Metadata md) throws Except
201200
final MutableInt status = new MutableInt(-1);
202201
byte[] content = new byte[0];
203202

204-
try (Page page = context.newPage()) {
205-
206-
page.onResponse(
207-
response -> {
208-
// make sure that this applies to the main page
209-
if (response.url().equals(url)) {
210-
// redirection?
211-
if (Status.REDIRECTION.equals(
212-
Status.fromHTTPCode(response.status()))) {
213-
status.set(response.status());
214-
response.allHeaders()
215-
.forEach(
216-
(k, v) -> {
217-
responseMetaData.addValue(k, v);
218-
});
203+
try {
204+
try (Page page = context.newPage()) {
205+
206+
page.onResponse(
207+
response -> {
208+
// make sure that this applies to the main page
209+
if (response.url().equals(url)) {
210+
// redirection?
211+
if (Status.REDIRECTION.equals(
212+
Status.fromHTTPCode(response.status()))) {
213+
status.set(response.status());
214+
response.allHeaders()
215+
.forEach(
216+
(k, v) -> {
217+
responseMetaData.addValue(k, v);
218+
});
219+
}
219220
}
220-
}
221-
});
222-
223-
page.onPageError(
224-
handler -> {
225-
// this applies to any resource - not just the main page
226-
LOG.debug("Error when loading {} {}", url, handler);
227-
});
228-
229-
// NOTE: The handler will only be called for the first url if the
230-
// response is a redirect.
231-
page.route(
232-
lambdaUrl -> true,
233-
route -> {
234-
// abort if we know the main page is a redirection
235-
if (status.get() != -1) {
236-
LOG.debug("Aborting request for {}", route.request().url());
237-
route.abort();
238-
} else if (resourceTypesToSkip.contains(
239-
route.request().resourceType())) {
240-
route.abort();
241-
} else {
242-
route.resume();
243-
}
244-
});
245-
246-
// let playwright do the content loading
247-
com.microsoft.playwright.Response response =
248-
page.navigate(
249-
url,
250-
new Page.NavigateOptions()
251-
.setTimeout(timeout)
252-
.setWaitUntil(loadEvent));
253-
254-
// the status is not set unless
255-
// a redirection
256-
if (status.get() == -1) {
257-
response.allHeaders()
258-
.forEach(
259-
(k, v) -> {
260-
responseMetaData.addValue(k, v);
261-
});
262-
263-
int httpStatus = response.status();
264-
boolean fetched = Status.FETCHED == Status.fromHTTPCode(httpStatus);
265-
boolean contentCaptured = false;
266-
267-
if (fetched || captureContentOnError) {
268-
// run any configured post-navigate actions before capturing content
269-
pageActions.apply(page, url, md, responseMetaData);
270-
// retrieve the rendered content
271-
content = page.content().getBytes(StandardCharsets.UTF_8);
272-
contentCaptured = true;
273-
}
221+
});
222+
223+
page.onPageError(
224+
handler -> {
225+
// this applies to any resource - not just the main page
226+
LOG.debug("Error when loading {} {}", url, handler);
227+
});
228+
229+
// NOTE: The handler will only be called for the first url if the
230+
// response is a redirect.
231+
page.route(
232+
lambdaUrl -> true,
233+
route -> {
234+
// abort if we know the main page is a redirection
235+
if (status.get() != -1) {
236+
LOG.debug("Aborting request for {}", route.request().url());
237+
route.abort();
238+
} else if (resourceTypesToSkip.contains(
239+
route.request().resourceType())) {
240+
route.abort();
241+
} else {
242+
route.resume();
243+
}
244+
});
245+
246+
// let playwright do the content loading
247+
com.microsoft.playwright.Response response =
248+
page.navigate(
249+
url,
250+
new Page.NavigateOptions()
251+
.setTimeout(timeout)
252+
.setWaitUntil(loadEvent));
253+
254+
// the status is not set unless
255+
// a redirection
256+
if (status.get() == -1) {
257+
response.allHeaders()
258+
.forEach(
259+
(k, v) -> {
260+
responseMetaData.addValue(k, v);
261+
});
262+
263+
int httpStatus = response.status();
264+
boolean fetched = Status.FETCHED == Status.fromHTTPCode(httpStatus);
265+
boolean contentCaptured = false;
266+
267+
if (fetched || captureContentOnError) {
268+
// run any configured post-navigate actions before capturing content
269+
pageActions.apply(page, url, md, responseMetaData);
270+
// retrieve the rendered content
271+
content = page.content().getBytes(StandardCharsets.UTF_8);
272+
contentCaptured = true;
273+
}
274274

275-
if (!fetched && contentCaptured && overrideStatusOnContent) {
276-
// expose the original origin status for diagnostics
277-
responseMetaData.setValue(
278-
"playwright.origin.status", Integer.toString(httpStatus));
279-
status.set(200);
280-
} else {
281-
status.set(httpStatus);
282-
}
275+
if (!fetched && contentCaptured && overrideStatusOnContent) {
276+
// expose the original origin status for diagnostics
277+
responseMetaData.setValue(
278+
"playwright.origin.status", Integer.toString(httpStatus));
279+
status.set(200);
280+
} else {
281+
status.set(httpStatus);
282+
}
283283

284-
// evaluate an expression and store the results
285-
// in the metadata using the same string as key
286-
for (String expression : evaluations) {
287-
Object performance = page.evaluate(expression);
288-
if (performance != null) {
289-
String json =
290-
mapper.writerWithDefaultPrettyPrinter()
291-
.writeValueAsString(performance);
292-
responseMetaData.setValue(expression, json);
284+
// evaluate an expression and store the results
285+
// in the metadata using the same string as key
286+
for (String expression : evaluations) {
287+
Object performance = page.evaluate(expression);
288+
if (performance != null) {
289+
String json =
290+
mapper.writerWithDefaultPrettyPrinter()
291+
.writeValueAsString(performance);
292+
responseMetaData.setValue(expression, json);
293+
}
293294
}
294295
}
295296
}
296-
}
297297

298-
if (md.containsKey(MD_TRACE)) {
299-
Path tmp = Files.createTempFile("trace-", ".zip", new FileAttribute[0]);
300-
context.tracing().stop(new Tracing.StopOptions().setPath(tmp));
301-
responseMetaData.setValue(MD_TRACE, tmp.toString());
302-
}
298+
if (isTracing) {
299+
Path tmp = Files.createTempFile("trace-", ".zip", new FileAttribute[0]);
300+
context.tracing().stop(new Tracing.StopOptions().setPath(tmp));
301+
responseMetaData.setValue(MD_TRACE, tmp.toString());
302+
}
303303

304-
responseMetaData.addValue(MD_KEY_END, Instant.now().toString());
304+
responseMetaData.addValue(MD_KEY_END, Instant.now().toString());
305305

306-
return new ProtocolResponse(content, status.get(), responseMetaData);
306+
return new ProtocolResponse(content, status.get(), responseMetaData);
307+
308+
} finally {
309+
if (isTracing && responseMetaData.getFirstValue(MD_TRACE) == null) {
310+
try {
311+
context.tracing().stop(new Tracing.StopOptions());
312+
} catch (Exception e) {
313+
LOG.warn("Exception while stopping tracing on error", e);
314+
}
315+
}
316+
}
307317
}
308318
}
309319

0 commit comments

Comments
 (0)