Skip to content

Commit

Permalink
Fix SystemCallError message when errno is unknown
Browse files Browse the repository at this point in the history
  • Loading branch information
andrykonchin committed Dec 5, 2024
1 parent 431177d commit 7d8b161
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 33 deletions.
10 changes: 8 additions & 2 deletions spec/ruby/core/exception/system_call_error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,17 @@ def initialize

it "treats nil custom message as if it is not passed at all" do
exc = SystemCallError.new(nil, @example_errno)
exc.should be_an_instance_of(@example_errno_class)
exc.errno.should == @example_errno
exc.message.should == 'Invalid argument'
end

it "sets an 'unknown error' message when an unknown error number" do
SystemCallError.new(-1).message.should =~ /Unknown error(:)? -1/
end

it "adds a custom error message to an 'unknown error' message when an unknown error number and a custom message specified" do
SystemCallError.new("custom message", -1).message.should =~ /Unknown error(:)? -1 - custom message/
end

it "converts to Integer if errno is a Complex convertible to Integer" do
SystemCallError.new('foo', Complex(2.9, 0)).should == SystemCallError.new('foo', 2)
end
Expand Down
20 changes: 16 additions & 4 deletions spec/ruby/optional/capi/kernel_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,27 +187,39 @@ class CApiKernelSpecs::Exc < StandardError
it "raises an exception from the given error" do
-> do
@s.rb_syserr_fail(Errno::EINVAL::Errno, "additional info")
end.should raise_error(Errno::EINVAL, /additional info/)
end.should raise_error(Errno::EINVAL, "Invalid argument - additional info")
end

it "can take a NULL message" do
-> do
@s.rb_syserr_fail(Errno::EINVAL::Errno, nil)
end.should raise_error(Errno::EINVAL)
end.should raise_error(Errno::EINVAL, "Invalid argument")
end

it "uses an 'unknown error' message when errno is unknown" do
-> do
@s.rb_syserr_fail(-10, nil)
end.should raise_error(SystemCallError, /Unknown error(:)? -1/) # a new class Errno::E-01 is generated on the fly
end
end

describe "rb_syserr_fail_str" do
it "raises an exception from the given error" do
-> do
@s.rb_syserr_fail_str(Errno::EINVAL::Errno, "additional info")
end.should raise_error(Errno::EINVAL, /additional info/)
end.should raise_error(Errno::EINVAL, "Invalid argument - additional info")
end

it "can take nil as a message" do
-> do
@s.rb_syserr_fail_str(Errno::EINVAL::Errno, nil)
end.should raise_error(Errno::EINVAL)
end.should raise_error(Errno::EINVAL, "Invalid argument")
end

it "uses an 'unknown error' message when errno is unknown" do
-> do
@s.rb_syserr_fail_str(-1, nil)
end.should raise_error(SystemCallError, /Unknown error(:)? -1/) # a new class Errno::E-01 is generated on the fly
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/cext/CExtNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,7 @@ public abstract static class RbSysErrFail extends CoreMethodArrayArgumentsNode {
Object rbSysErrFail(int errno, Object string,
@Cached ErrnoErrorNode errnoErrorNode) {
final Backtrace backtrace = getContext().getCallStack().getBacktrace(this);
throw new RaiseException(getContext(), errnoErrorNode.execute(null, errno, string, backtrace));
throw new RaiseException(getContext(), errnoErrorNode.execute(null, errno, string, null, backtrace));
}

}
Expand Down
22 changes: 14 additions & 8 deletions src/main/java/org/truffleruby/core/exception/ErrnoErrorNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.truffleruby.core.klass.RubyClass;
import org.truffleruby.core.string.RubyString;
import org.truffleruby.core.string.ImmutableRubyString;
import org.truffleruby.language.Nil;
import org.truffleruby.language.RubyBaseNode;
import org.truffleruby.language.backtrace.Backtrace;
import org.truffleruby.language.dispatch.DispatchNode;
Expand All @@ -33,10 +34,11 @@ public static ErrnoErrorNode create() {
@Child private DispatchNode formatMessageNode;

public abstract RubySystemCallError execute(RubyClass rubyClass, int errno, Object extraMessage,
Backtrace backtrace);
Object location, Backtrace backtrace);

@Specialization
RubySystemCallError errnoError(RubyClass rubyClass, int errno, Object extraMessage, Backtrace backtrace,
RubySystemCallError errnoError(
RubyClass rubyClass, int errno, Object extraMessage, Object location, Backtrace backtrace,
@Cached TruffleString.FromJavaStringNode fromJavaStringNode) {
final String errnoName = getContext().getCoreLibrary().getErrnoName(errno);

Expand All @@ -57,26 +59,30 @@ RubySystemCallError errnoError(RubyClass rubyClass, int errno, Object extraMessa
Encodings.UTF_8);
}

final RubyString errorMessage = formatMessage(errnoDescription, errno, extraMessage);
final RubyString errorMessage = formatMessage(errnoDescription, errno, extraMessage, location);

return ExceptionOperations
.createSystemCallError(getContext(), errnoClass, errorMessage, errno, backtrace);
}

private RubyString formatMessage(Object errnoDescription, int errno, Object extraMessage) {
assert extraMessage instanceof RubyString || extraMessage instanceof ImmutableRubyString;
private RubyString formatMessage(Object errnoDescription, int errno, Object extraMessage, Object location) {
assert extraMessage instanceof RubyString || extraMessage instanceof ImmutableRubyString ||
extraMessage instanceof Nil;

if (formatMessageNode == null) {
CompilerDirectives.transferToInterpreterAndInvalidate();
formatMessageNode = insert(DispatchNode.create());
}

if (location == null) {
location = nil;
}

var arguments = new Object[]{ errnoDescription, errno, extraMessage, location };
return (RubyString) formatMessageNode.call(
getContext().getCoreLibrary().truffleExceptionOperationsModule,
"format_errno_error_message",
errnoDescription,
errno,
extraMessage);
arguments);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ public abstract static class ExceptionErrnoErrorPrimitiveNode extends PrimitiveA
@Child ErrnoErrorNode errnoErrorNode = ErrnoErrorNode.create();

@Specialization
RubySystemCallError exceptionErrnoError(RubyClass errorClass, Object message, int errno) {
return errnoErrorNode.execute(errorClass, errno, message, null);
RubySystemCallError exceptionErrnoError(RubyClass errorClass, Object extraMessage, int errno, Object location) {
return errnoErrorNode.execute(errorClass, errno, extraMessage, location, null);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,11 +546,11 @@ public static TStringBuilder formatToTStringBuilder(Token[] compiledPattern, Zon
} catch (IndexOutOfBoundsException ioobe) {
final Backtrace backtrace = context.getCallStack().getBacktrace(currentNode);
final RubyString message = StringOperations.createUTF8String(context, language, "strftime");
final int errno = context.getCoreLibrary().getErrnoValue("ERANGE");

throw new RaiseException(
context,
errnoErrorNode.execute(null, context.getCoreLibrary().getErrnoValue("ERANGE"), message,
backtrace));
errnoErrorNode.execute(null, errno, message, null, backtrace));
}

// reset formatter
Expand Down
10 changes: 2 additions & 8 deletions src/main/ruby/truffleruby/core/exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,6 @@ def success?

class SystemCallError < StandardError

def self.errno_error(klass, message, errno, location)
message = message ? " - #{message}" : +''
message = " @ #{location}#{message}" if location
Primitive.exception_errno_error klass, message, errno
end

# We use .new here because when errno is set, we attempt to
# lookup and return a subclass of SystemCallError, specifically,
# one of the Errno subclasses.
Expand Down Expand Up @@ -315,7 +309,7 @@ def self.new(*args)
# If it corresponds to a known Errno class, create and return it now
if errno
errno = Primitive.rb_num2long(errno)
error = SystemCallError.errno_error(self, message, errno, location)
error = Primitive.exception_errno_error(self, message, errno, location)
return error unless Primitive.nil? error
end
super(message, errno, location)
Expand All @@ -334,7 +328,7 @@ def self.new(*args)
end

if defined?(self::Errno) && Primitive.is_a?(self::Errno, Integer)
error = SystemCallError.errno_error(self, message, self::Errno, location)
error = Primitive.exception_errno_error(self, message, self::Errno, location)
if error && Primitive.equal?(Primitive.class(error), self)
return error
end
Expand Down
11 changes: 5 additions & 6 deletions src/main/ruby/truffleruby/core/truffle/exception_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,11 @@ def self.comparison_error_message(x, y)
format("super: no superclass method `%s'", exception.name)
end

def self.format_errno_error_message(errno_description, errno, extra_message)
if Primitive.nil? errno_description
"unknown error: #{errno} - #{extra_message}"
else
"#{errno_description}#{extra_message}"
end
def self.format_errno_error_message(errno_description, errno, extra_message, location)
message = errno_description || "Unknown error: #{errno}"
message += " @ #{location}" if location
message += " - #{extra_message}" if extra_message
+message
end
end
end

0 comments on commit 7d8b161

Please sign in to comment.