sdk calls are not url encoding path parameters

[perrytew]

Hey TaxJar,

I discovered an issue with the sdk when trying to do an update to a refund. It was blowing up with an unparsable exception (trace below). I was using a transaction id of our order id concatenated with the characters " [REFUND]" (notice the leading space). This worked for the create refund since that was a POST and the transaction id was part of the json payload. But for the subsequent PUT call made by the api.updateRefund, it blew up because the transaction id was a path parameter.

When I changed my scheme to order id + "-REFUND" (no spaces or brackets), it worked without issue.

I'm proceeding without issue, so this isn't a showstopper for me. But you may wish to url encode all path parameters to avoid any future issues.

Finally, it would be most helpful if you trapped the following exception: java.lang.NullPointerException at com.taxjar.exception.TaxjarException.parseMessage(TaxjarException.java:28) at com.taxjar.exception.TaxjarException.<init>(TaxjarException.java:15) at com.taxjar.exception.TaxjarException.<init>(TaxjarException.java:11) at com.taxjar.net.ApiRequest.execute(ApiRequest.java:23) at com.taxjar.Taxjar.updateRefund(Taxjar.java:296)

This was happening when the sdk attempted to parse the response coming back from the server. Since my request was blowing up, I'm assuming your server was sending back a 404 or a 500 error. If the parseMessage fails, it would be super helpful if you trapped that failure and threw an exception with the http status code and response body.

If the sdk had done such a thing, my troubleshooting would have accelerated. As soon as I saw a 404 or 500 with a message of "no such endpoint", etc, I would have immediately started looking at the request construction.

Just some thoughts. Great API. We've enjoyed using it. Thanks again for providing it. Cheers, Perry Tew

[ScottRudiger]

Thanks for the detailed issue @perrytew! We'll get this fixed up in due time.

[ScottRudiger]

@perrytew Would you mind sharing the request or a code sample of where you hit the NullPointerException? In my testing, prior to the changes in #24, a 500 error was returned from the API when updating a refund with a space in the transaction_id.

[perrytew]

Scott, I didn't see 24. From the description, I'm sure this is a duplicate of that issue. Please close it as such. My apologies for any time wasted. I did not realize we were not on the most recent version.

[ScottRudiger]

@perrytew, your issue came first. That's a PR to address the issue (not merged yet), opened thanks to you! So far, the PR only covers encoding path parameters, such as transaction_id in UpdateRefund properly.

I wasn't able to recreate the NullPointerException you got, however. Just wanted to make sure that's addressed as well.

[perrytew]

Okay, here's a reproduction of the error. Should be easy for you to replicate.

  @Test
  public void forceRefundErrorForGithubIssue() throws TaxjarException {
    Taxjar client = new Taxjar(SANDBOX_API_TOKEN);
    Map<String, Object> params = new HashMap<>(); // empty params is fine.
    String refundId = "ORDER-12345 [REFUND]"; // space here forces NPE on next line
    RefundResponse refundResponse = client.updateRefund(refundId, params);
  }

Result:

java.lang.NullPointerException
    at com.taxjar.exception.TaxjarException.parseMessage(TaxjarException.java:28)
    at com.taxjar.exception.TaxjarException.<init>(TaxjarException.java:15)
    at com.taxjar.exception.TaxjarException.<init>(TaxjarException.java:11)
    at com.taxjar.net.ApiRequest.execute(ApiRequest.java:23)
    at com.taxjar.Taxjar.updateRefund(Taxjar.java:296)
    at com.bps.ultracart.engine.taxes.api.TaxJarApiTest.forceRefundErrorForGithubIssue(TaxJarApiTest.java:1244)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:80)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:691)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:883)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1208)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:127)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:111)
    at org.testng.TestRunner.privateRun(TestRunner.java:753)
    at org.testng.TestRunner.run(TestRunner.java:613)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:334)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:329)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:291)
    at org.testng.SuiteRunner.run(SuiteRunner.java:240)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1137)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1062)
    at org.testng.TestNG.run(TestNG.java:974)
    at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:66)
    at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:110)

Hope this helps. Thanks Scott.