fix(r/adbcdrivermanager): don't truncate long error messages by ump45nose · Pull Request #4939 · apache/arrow-adbc

ump45nose · 2026-10-21T15:05:35Z

Closes #3450.

R truncates the printed message of an uncaught error to getOption("warning.length") characters, which is 1000 by default. Driver manager errors can easily be longer than that, for example when a driver isn't found and the message lists every dlopen() attempt and every path searched for manifests, so the end of the message (the part listing the manifest search paths) gets cut off in R while Python shows all of it.

stop_for_error() now raises warning.length to its maximum (8170) while it signals the condition, but only when the message is longer than the current limit. The option is restored via on.exit() once stop() unwinds, so the user's setting is left unchanged. The condition object itself was never truncated (conditionMessage(e) already had the full text); this only affects what is printed.

This PR was prepared with AI assistance.

R truncates printed error messages to getOption("warning.length") (1000 characters by default), so long driver manager messages such as the list of paths searched for manifests were cut off. Raise the limit while stop_for_error() signals the condition. Closes apache#3450.

amoeba

Thanks for this. Just one suggestion.

amoeba · 2026-10-21T15:36:26Z

+    if (nchar(msg, type = "bytes") > getOption("warning.length", 1000L)) {
+      old_options <- options(warning.length = 8170L)
+      on.exit(options(old_options), add = TRUE)
+    }
+


There's a slight bug in here. The truncation counts the entire error message which includes the "Error in adbc_driver("some_driver") :" prefix to msg above. So if we had a 1000-byte msg, I think it would get truncated because R counts the entire thing.

I think we could just always raise the limit since it's cheap and I don't think we want to implement the logic to figure out how long the "Error in..." message will be:

Suggested change

if (nchar(msg, type = "bytes") > getOption("warning.length", 1000L)) {

old_options <- options(warning.length = 8170L)

on.exit(options(old_options), add = TRUE)

}

old_options <- options(warning.length = 8170L)

on.exit(options(old_options), add = TRUE)

and update the comment above.

Good catch, thanks. The limit covers the whole printed message including the Error in ... : prefix, so the length check was wrong. I've applied your suggestion so warning.length is always raised to 8170 while the error is signalled, and updated the comment above it (579d734). The existing test still checks that the option is 8170 while signalled and restored afterwards. I couldn't run the R tests locally this time, so I'm relying on CI for that.

…g errors

Copilot

🟡 Changes recommended

The new test can abort R when manifest search directories exist because its synthetic driver name exceeds filesystem limits.

1 open issue

Avoid filesystem errors from oversized synthetic driver names · New

What changed in this PR

This PR addresses #3450 by increasing R’s error-printing limit while the ADBC driver manager signals errors, allowing more diagnostic details to remain visible.

Changes:

Temporarily set warning.length to 8170 when signalling an error, then restore the previous setting.
Add a long-message test that checks the limit during signalling and its restoration afterward.

File	Description
`r/adbcdrivermanager/tests/testthat/test-error.R`	Tests long-error handling and option restoration.
`r/adbcdrivermanager/R/error.R`	Temporarily raises the error-printing limit.

_{Review effort: Balanced}

lidavidm · 2026-10-22T05:21:14Z

+  had_error <- FALSE
+  tryCatch(
+    withCallingHandlers(
+      adbc_driver(paste0("not_a_driver_", strrep("x", 1000))),


Hmm, sounds like the driver manager should be catching this (we can handle that separately)

I'm working on this in #4995.

Thanks! The test needs the printed message to exceed 1000 bytes, which the long name currently guarantees. Once #4995 lands I can shorten the name to a valid filename component (e.g. under 255 bytes) and check the message length still exceeds the default limit, or rebase onto it if you prefer. (AI-assisted reply.)

amoeba

Thanks for the patience. Just one change before merge here. Otherwise this looks good.

amoeba · 2026-11-04T23:38:03Z

+    # R truncates printed error messages to getOption("warning.length")
+    # characters (1000 by default), which cuts off long driver manager
+    # messages such as the list of paths searched for a driver manifest.
+    # The limit applies to the whole printed message, including the
+    # "Error in <call> :" prefix, so always raise it to its maximum while
+    # the error is signalled; the option is restored when stop() unwinds.


Suggested change

# R truncates printed error messages to getOption("warning.length")

# characters (1000 by default), which cuts off long driver manager

# messages such as the list of paths searched for a driver manifest.

# The limit applies to the whole printed message, including the

# "Error in <call> :" prefix, so always raise it to its maximum while

# the error is signalled; the option is restored when stop() unwinds.

# Always temporarily raise warning.length to its maximum so driver manager

# errors don't get truncated

This comment is unnecessarily verbose. How about this?

Agreed, applied in fd8cd23. (AI-assisted reply.)

amoeba · 2026-11-04T23:43:17Z

+  had_error <- FALSE
+  tryCatch(
+    withCallingHandlers(
+      adbc_driver(paste0("not_a_driver_", strrep("x", 1000))),


Probably best to bump this down so it's always a valid filename.

Signed-off-by: Vincent <52391318+ump45nose@users.noreply.github.com>

Copilot review on #4939 (comment) found an edge case in the C++ driver manager when strings that are too long to be valid operating-system-specific file paths are used. The current implementation uses the variant of `std::filesystem::exists` that throws and we weren't catching those. This PR converts those uses to `bool exists( const std::filesystem::path& p, std::error_code& ec ) noexcept;` and either ignores the error or handles it an enhances the driver manager error. This PR was detected with Copilot and I drove Opus 5.5 around. The changes were tested in CI, see comments in the PR thread.

ump45nose requested a review from paleolimbot as a code owner October 21, 2026 15:05

amoeba reviewed Oct 21, 2026

View reviewed changes

fix(r/adbcdrivermanager): always raise warning.length while signallin…

579d734

…g errors

lidavidm requested a balanced review from Copilot October 22, 2026 04:59

Copilot started reviewing on behalf of lidavidm October 22, 2026 05:00

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

amoeba requested changes Nov 4, 2026

View reviewed changes

amoeba reviewed Nov 4, 2026

View reviewed changes

amoeba mentioned this pull request Nov 5, 2026

fix(c/driver_manager): handle long filenames instead of throwing #4995

Merged

fix(r/adbcdrivermanager): shorten warning.length comment

fd8cd23

Signed-off-by: Vincent <52391318+ump45nose@users.noreply.github.com>

Navigation Menu

Repository navigation

fix(r/adbcdrivermanager): don't truncate long error messages - #4939

fix(r/adbcdrivermanager): don't truncate long error messages#4939
ump45nose wants to merge 3 commits into
apache:mainfrom
ump45nose:fix/3450-r-error-truncation

ump45nose commented Oct 21, 2026 •

edited by lidavidm

Loading

Uh oh!

amoeba left a comment

Uh oh!

amoeba Oct 21, 2026

Uh oh!

ump45nose Oct 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lidavidm Oct 22, 2026

Uh oh!

amoeba Nov 5, 2026

Uh oh!

ump45nose Nov 5, 2026

Uh oh!

amoeba left a comment

Uh oh!

amoeba Nov 4, 2026

Uh oh!

ump45nose Nov 5, 2026

Uh oh!

amoeba Nov 4, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ump45nose commented Oct 21, 2026 • edited by lidavidm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amoeba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amoeba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ump45nose commented Oct 21, 2026 •

edited by lidavidm

Loading