csvlint.io icon indicating copy to clipboard operation
csvlint.io copied to clipboard

Report an error if schema isn't loaded

Open ldodds opened this issue 10 years ago • 10 comments

If a user supplies a schema and we fail to load it, then the application should report an error so its clear that the schema hasn't been applied.

Currently it falls back to not applying any schema validation which is misleading.

As a first step, reporting a basic error would be an immediate improvement.

Reporting why the schema is invalid would also be useful, although there are a range of possible errors:

  • invalid JSON
  • invalid regex
  • invalid values for a constraint specification, e.g. a number or string where true/false is expected
  • unknown constraint

The first two are definitely fatal errors. The last two are potentially just warnings. Depends if we want to work on a "best effort" basis.

ldodds avatar Jun 02 '15 20:06 ldodds

I'll be taking invalid regex

bcouston avatar Jul 09 '15 15:07 bcouston

change error message for schema error, currently prints "Sorry, your CSV did not pass validation. Please review the errors and warnings below:"

quadrophobiac avatar Jul 10 '15 09:07 quadrophobiac

lines 26 - 34 of Validation currently always eval false, would be worthwhile working out what condition they were intended to catch

    if io.respond_to?(:tempfile)
      filename = io.original_filename
      csv = File.new(io.tempfile)
      io = File.new(io.tempfile)
    elsif io.class == Hash && !io[:body].nil?
      filename = io[:filename]
      csv_id = io[:csv_id]
      io = StringIO.new(io[:body])
    end

quadrophobiac avatar Jul 10 '15 13:07 quadrophobiac

@quadrophobiac you mean both of these conditions always fail? The first looks like it's intended to handle uploaded files, and the second ... not sure. Will look at the context....

Floppy avatar Jul 10 '15 13:07 Floppy

@quadrophobiac do you mean this section? https://github.com/theodi/csvlint/blob/master/app/models/validation.rb#L19

The line numbers seems different...

Floppy avatar Jul 10 '15 13:07 Floppy

Yes apologies - I was referring to the line numbers on the error-reporting-schema_expanded-test-suite branch

quadrophobiac avatar Jul 10 '15 13:07 quadrophobiac

I've confirmed non triggering by running the tests suite with a byebug in each conditional

quadrophobiac avatar Jul 10 '15 13:07 quadrophobiac

Ah, I looked on the fix-schema branch but not the other - my bad. Looking closer now.

Floppy avatar Jul 10 '15 13:07 Floppy

Started a pull request on invalid regex functionality: https://github.com/theodi/csvlint.rb/pull/132

bcouston avatar Jul 14 '15 08:07 bcouston

theodi/csvlint.rb#132 is good to go - so we need to make sure the newly created errors are shown this end

pezholio avatar Jul 24 '15 12:07 pezholio