[chore] Fix unhandled exceptions thrown by stoi #48965

Tech-Circle-48 · 2025-03-12T08:34:16Z

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #48859

Problem Summary:
fixes the issue where providing a timeout value exceeding the range of int32 causes a core dump during streamload.

The root cause is that the timeout value parsed using std::stoi() can throw a std::out_of_range exception if the value exceeds int32 limits, which wasn't handled correctly.

Release note

None

Check List (For Author)

Test
- Regression test
- Unit Test
- Manual test (add detailed scripts or steps below)
- No need to test or manual test. Explain why:
  - This is a refactor/code format and no logic has been changed.
  - Previous test can cover this change.
  - No code files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

hello-stephen · 2025-03-12T08:34:22Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

sollhui · 2025-03-12T08:43:56Z

be/src/http/action/stream_load.cpp

@@ -337,6 +337,8 @@ Status StreamLoadAction::_on_header(HttpRequest* http_req, std::shared_ptr<Strea
            ctx->timeout_second = std::stoi(http_req->header(HTTP_TIMEOUT));
        } catch (const std::invalid_argument& e) {
            return Status::InvalidArgument("Invalid timeout format, {}", e.what());
+        } catch (const std::out_of_range& e) {


Please add regression test.

run buildall

liaoxin01 · 2025-03-12T14:48:44Z

be/src/runtime/stream_load/stream_load_context.h

@@ -157,7 +157,7 @@ class StreamLoadContext {
    // optional
    std::string sub_label;
    double max_filter_ratio = 0.0;
-    int32_t timeout_second = -1;
+    int64_t timeout_second = -1;


We don't need to change this type.

liaoxin01 · 2025-03-12T14:51:48Z

be/src/http/action/stream_load.cpp

@@ -337,6 +337,8 @@ Status StreamLoadAction::_on_header(HttpRequest* http_req, std::shared_ptr<Strea
            ctx->timeout_second = std::stoi(http_req->header(HTTP_TIMEOUT));
        } catch (const std::invalid_argument& e) {
            return Status::InvalidArgument("Invalid timeout format, {}", e.what());
+        } catch (const std::out_of_range& e) {


Please also fix other places that use the stoi function together.

Tech-Circle-48 · 2025-03-18T02:31:54Z

run buildall

dataroaring · 2025-03-18T02:37:22Z

be/src/exec/es/es_scan_reader.cpp

+        LOG(WARNING) << fmt::format("Invalid shard_id format: {}, error: {}", _shards, e.what());
+    } catch (std::out_of_range& e) {
+        LOG(WARNING) << fmt::format("Shard_id value out of range: {}, error: {}", _shards,
+                                    e.what());


set shard_id = -1 ?

dataroaring · 2025-03-18T02:39:08Z

be/src/olap/data_dir.cpp

+            } catch (std::invalid_argument&) {
+                LOG(WARNING) << "Invalid cluster_id value: " << content;
+            } catch (std::out_of_range&) {
+                LOG(WARNING) << "cluster_id value out of range: " << content;


just return an errror.

dataroaring · 2025-03-18T02:40:10Z

be/src/olap/rowset/segment_v2/inverted_index_reader.cpp

+                     << ", error: " << e.what();
+    } catch (const std::out_of_range& e) {
+        LOG(WARNING) << "ignore_above value out of range: "
+                     << ",error: " << e.what();


Maybe we should return an error.

dataroaring · 2025-03-18T02:41:32Z

be/src/olap/storage_engine.cpp

+                } catch (const std::invalid_argument& e) {
+                    LOG(WARNING) << "Invalid expiration value format in dir_name: " << dir_name
+                                 << ",error: " << e.what();
+                    actual_expire = 0;


do not set it to zero.

gavinchou · 2025-03-18T02:46:02Z

regression-test/suites/load_p0/stream_load/stream_load.cpp

why this cpp file is in regression-test folder?

already remove

gavinchou · 2025-03-18T02:49:23Z

cloud/src/meta-service/meta_service_resource.cpp

+                        return;
+                    }
+                } catch (const std::invalid_argument& e) {
+                    LOG(WARNING) << "Invalid idx format: " << s.obj_info().id()


do error handling as line 2782 does when we meet exception

It's been modified

gavinchou · 2025-03-18T02:55:19Z

cloud/src/meta-service/meta_service_resource.cpp

+                                 << ", error: " << e.what();
+                    return;
+                } catch (const std::out_of_range& e) {
+                    LOG(WARNING) << "Idx value out of range: " << s.obj_info().id()


It's been modified

doris-robot · 2025-03-18T06:01:45Z

BE UT Coverage Report

Increment line coverage 18.36% (38/207) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	48.88% (13092/26784)
Line Coverage	38.44% (112880/293689)
Region Coverage	37.26% (57411/154099)
Branch Coverage	32.34% (28837/89166)

airborne12 · 2025-03-19T06:38:14Z

be/src/olap/rowset/segment_v2/inverted_index_writer.cpp

+            _ignore_above = std::stoi(ignore_above_value);
+        } catch (const std::invalid_argument& e) {
+            LOG(WARNING) << "Invalid ignore_above_value format: " << ignore_above_value
+                         << ", using default, error: " << e.what();


need to return Status::ErrorErrorCode::INVERTED_INDEX_CLUCENE_ERROR()

fix1

2c45bb8

fix2

5721a96

sollhui reviewed Mar 12, 2025

View reviewed changes

liaoxin01 reviewed Mar 12, 2025

View reviewed changes

fix8

4fe2f82

Tech-Circle-48 requested review from gavinchou, dataroaring and w41ter as code owners March 18, 2025 02:25

dataroaring reviewed Mar 18, 2025

View reviewed changes

gavinchou reviewed Mar 18, 2025

View reviewed changes

remove

eaf2487

gavinchou changed the title ~~[fix](streamload) Fix streamload timeout out-of-range issue casing coredump~~ [chore] Fix unhandled exceptions thrown by stoi Mar 18, 2025

gavinchou reviewed Mar 18, 2025

View reviewed changes

fix9

9c7f1a4

modify error

3a70741

airborne12 reviewed Mar 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[chore] Fix unhandled exceptions thrown by stoi #48965

[chore] Fix unhandled exceptions thrown by stoi #48965

Tech-Circle-48 commented Mar 12, 2025

hello-stephen commented Mar 12, 2025

sollhui Mar 12, 2025 •

edited

Loading

Tech-Circle-48 Mar 12, 2025

liaoxin01 Mar 12, 2025

liaoxin01 Mar 12, 2025

Tech-Circle-48 commented Mar 18, 2025

dataroaring Mar 18, 2025

qidaye Mar 18, 2025

dataroaring Mar 18, 2025

dataroaring Mar 18, 2025

dataroaring Mar 18, 2025

gavinchou Mar 18, 2025 •

edited

Loading

Tech-Circle-48 Mar 18, 2025

gavinchou Mar 18, 2025

Tech-Circle-48 Mar 18, 2025

gavinchou Mar 18, 2025

Tech-Circle-48 Mar 18, 2025

doris-robot commented Mar 18, 2025

airborne12 Mar 19, 2025

[chore] Fix unhandled exceptions thrown by stoi #48965

Are you sure you want to change the base?

[chore] Fix unhandled exceptions thrown by stoi #48965

Conversation

Tech-Circle-48 commented Mar 12, 2025

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

hello-stephen commented Mar 12, 2025

sollhui Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tech-Circle-48 commented Mar 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavinchou Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doris-robot commented Mar 18, 2025

BE UT Coverage Report

Choose a reason for hiding this comment

sollhui Mar 12, 2025 •

edited

Loading

gavinchou Mar 18, 2025 •

edited

Loading