Skip to content

GH-33823: [C++] Improve error messages when opening files that are the wrong format#49771

Open
RobertLD wants to merge 5 commits intoapache:mainfrom
RobertLD:33823-improve-err-msg-for-file-reader
Open

GH-33823: [C++] Improve error messages when opening files that are the wrong format#49771
RobertLD wants to merge 5 commits intoapache:mainfrom
RobertLD:33823-improve-err-msg-for-file-reader

Conversation

@RobertLD
Copy link
Copy Markdown

@RobertLD RobertLD commented Apr 16, 2026

Rationale for this change

The original error messages did not provide instruction to users on how to best correct their usage

What changes are included in this PR?

Improving the messages

Are these changes tested?

Current unit tests were executed, no new tests were introduced

Are there any user-facing changes?

Yes, error messages have been updated that are surfaced to users

@github-actions
Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@RobertLD RobertLD changed the title #33823 Improve err msgs when opening files that are the wrong format GH-33823: [C++] Improve err msgs when opening files that are the wrong format Apr 16, 2026
@RobertLD RobertLD force-pushed the 33823-improve-err-msg-for-file-reader branch from d7f5423 to abd2b9c Compare April 16, 2026 12:48
@RobertLD RobertLD marked this pull request as ready for review April 16, 2026 14:04
@kou kou requested a review from Copilot April 19, 2026 21:12
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves user-facing error messages in the C++ IPC readers/decoders when callers attempt to open an IPC File as a Stream (or vice versa), providing more actionable guidance aligned with GH-33823.

Changes:

  • Update the IPC file reader footer/magic check failure message to suggest the streaming reader when appropriate.
  • Add a heuristic in the IPC stream message decoder to detect an IPC file magic prefix being misinterpreted as a stream message length, and emit a more instructive error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
cpp/src/arrow/ipc/reader.cc Improves the “not an Arrow file” error when the footer magic doesn’t match, suggesting the streaming reader.
cpp/src/arrow/ipc/message.cc Adds detection for IPC File magic bytes when decoding a stream message and returns a more targeted “wrong format” error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cpp/src/arrow/ipc/reader.cc Outdated
Comment thread cpp/src/arrow/ipc/message.cc Outdated
@RobertLD
Copy link
Copy Markdown
Author

RobertLD commented Apr 19, 2026

Decent feedback Mr. Bot, I'll make changes shortly. Honestly impressed the LLM review had such good context. Good bot

RobertLD and others added 2 commits April 24, 2026 10:41
Create more generic message instead of referencing c++ interface

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Similarly, keep messages more generic and not referencing the c++ interface

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@RobertLD
Copy link
Copy Markdown
Author

@kou Comments addressed

@kou kou requested a review from Copilot April 28, 2026 02:56
@kou kou changed the title GH-33823: [C++] Improve err msgs when opening files that are the wrong format GH-33823: [C++] Improve error messages when opening files that are the wrong format Apr 28, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kou
Copy link
Copy Markdown
Member

kou commented Apr 28, 2026

Could you add tests for these cases?

We want to ensure we're providing advice to the user that is correct/actionable.
@RobertLD
Copy link
Copy Markdown
Author

RobertLD commented Apr 29, 2026

Added a pos/neg case, kept the string matching minimal so the tests weren't as flakey is the messages get reworked in another way in the future. If it's standard practice to match the entire err message I can make that change over a subset match @kou

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants