Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Handle LOCAL INFILE read errors #56

Merged
merged 3 commits into from May 27, 2019

Conversation

sehrope
Copy link
Contributor

@sehrope sehrope commented May 25, 2019

Adds an error event listener to the LOCAL INFILE read stream to handle read errors generated midway through reading the file. Also removes the previous fs.access(...) check as there is a race condition between validating whether the file exists and is readable and actually trying to read it.

If the file does not exist, is not readable, or some other error occurs while trying to read it then they will all be handled the same way.

This should handle the situations described in https://jira.mariadb.org/browse/CONJS-79

I haven't included a test of an intermittent failure by mocking the error mid read (it's a bit more complicated than I have time for right now) but this changes passes all the existing LOCAL INFILE tests including the missing file ones.

Would be good for someone to spot check the return values and error situations as I'm not too familiar with the code base so inferred the error handling based on how it was handled in the previous async access check.

rusher and others added 3 commits May 20, 2019 10:49
Adds an error event listener to the LOCAL INFILE read stream to handle read
errors generated midway through reading the file. Also removes the previous
fs.access(...) check as there is a race condition between validating whether
the file exists and is readable and actually trying to read it.

If the file does not exist, is not readable, or some other error occurs while
trying to read it then they will all be handled the same way.
@codecov-io
Copy link

codecov-io commented May 25, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@4bf9a84). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop      #56   +/-   ##
==========================================
  Coverage           ?   95.96%           
==========================================
  Files              ?       52           
  Lines              ?     6266           
  Branches           ?        0           
==========================================
  Hits               ?     6013           
  Misses             ?      253           
  Partials           ?        0
Impacted Files Coverage Δ
lib/connection.js 94.9% <100%> (ø)
lib/cmd/resultset.js 93.75% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bf9a84...1987c72. Read the comment docs.

@rusher rusher merged commit 1987c72 into mariadb-corporation:develop May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants